From 1e314e09d00e14dc6b5b3b657432e85d17d1c2d2 Mon Sep 17 00:00:00 2001 From: Scott Lamb Date: Wed, 21 Apr 2021 10:44:01 -0700 Subject: [PATCH] refine timestamps in json signals api * API change: in update signals, allow setting a start time relative to now. This is an accuracy improvement in the case where the client has been retrying an initial request for a while. Kind of an obscure corner case but easy enough to address. And use a more convenient enum representation. * in update signals, choose `now` before acquiring the database lock. If lock acquisition takes a long time, this more accurately reflects the time the caller intended. * in general, make Time and Duration (de)serializable and use them in json types. This makes the types more self-describing, with better debug printing on both the server side and on the client library (in moonfire-playground). To make this work, base has to import serde which initially seemed like poor layering to me, but serde seems to be imported in some pretty foundational Rust crates for this reason. I'll go with it. --- design/api.md | 25 ++++++++++----------- server/Cargo.lock | 2 ++ server/base/Cargo.toml | 2 ++ server/base/time.rs | 12 ++++++++-- server/src/json.rs | 50 +++++++++++++++++++++--------------------- server/src/web.rs | 23 +++++++++---------- 6 files changed, 61 insertions(+), 53 deletions(-) diff --git a/design/api.md b/design/api.md index 71b11d0..95f0162 100644 --- a/design/api.md +++ b/design/api.md @@ -658,13 +658,12 @@ make. It should be a dict with these attributes: * `signalIds`: a list of signal ids to change. Must be sorted. * `states`: a list (one per `signalIds` entry) of states to set. -* `startTime90k`: (optional) The start of the observation in 90 kHz units - since 1970-01-01 00:00:00 UTC; commonly taken from an earlier response. If - absent, assumed to be now. -* `endBase`: if `epoch`, `relEndTime90k` is relative to 1970-01-01 00:00:00 - UTC. If `now`, epoch is relative to the current time. -* `relEndTime90k` (optional): The end of the observation, relative to the - specified base. Note this time is allowed to be in the future. +* `start`: the starting time of the change, as a dict of the form + `{'base': 'epoch', 'rel90k': t}` or `{'base': 'now', 'rel90k': t}`. In + the `epoch` form, `rel90k` is 90 kHz units since 1970-01-01 00:00:00 UTC. + In the `now` form, `rel90k` is relative to current time and may be + negative. +* `end`: the ending time of the change, in the same form as `start`. The response will be an `application/json` body dict with the following attributes: @@ -687,8 +686,8 @@ Request: { "signalIds": [1], "states": [2], - "endBase": "now", - "relEndTime90k": 5400000 + "start": {"base": "now", "rel90k": 0}, + "end": {"base": "now", "rel90k": 5400000} } ``` @@ -711,8 +710,8 @@ Request: { "signalIds": [1], "states": [2], - "endBase": "now", - "relEndTime90k": 5400000 + "start": {"base": "epoch", "rel90k": 140067468000000}, + "end": {"base": "now", "rel90k": 5400000} } ``` @@ -735,8 +734,8 @@ Request: { "signalIds": [1], "states": [2], - "endBase": "now", - "relEndTime90k": 5400000 + "start": {"base": "now", "rel90k": 0}, + "end": {"base": "now", "rel90k": 5400000} } } ``` diff --git a/server/Cargo.lock b/server/Cargo.lock index 3251056..d2cf040 100644 --- a/server/Cargo.lock +++ b/server/Cargo.lock @@ -1165,6 +1165,8 @@ dependencies = [ "log", "nom", "parking_lot", + "serde", + "serde_json", "time", ] diff --git a/server/base/Cargo.toml b/server/base/Cargo.toml index 6fc53b6..135ba9f 100644 --- a/server/base/Cargo.toml +++ b/server/base/Cargo.toml @@ -19,4 +19,6 @@ libc = "0.2" log = "0.4" parking_lot = { version = "0.11.1", features = [] } nom = "6.0.0" +serde = { version = "1.0", features = ["derive"] } +serde_json = "1.0" time = "0.1" diff --git a/server/base/time.rs b/server/base/time.rs index e6e6cd8..e3959eb 100644 --- a/server/base/time.rs +++ b/server/base/time.rs @@ -9,6 +9,7 @@ use nom::branch::alt; use nom::bytes::complete::{tag, take_while_m_n}; use nom::combinator::{map, map_res, opt}; use nom::sequence::{preceded, tuple}; +use serde::{Deserialize, Serialize}; use std::fmt; use std::ops; use std::str::FromStr; @@ -19,7 +20,7 @@ type IResult<'a, I, O> = nom::IResult>; pub const TIME_UNITS_PER_SEC: i64 = 90_000; /// A time specified as 90,000ths of a second since 1970-01-01 00:00:00 UTC. -#[derive(Clone, Copy, Default, Eq, Ord, PartialEq, PartialOrd)] +#[derive(Clone, Copy, Default, Deserialize, Eq, Ord, PartialEq, PartialOrd, Serialize)] pub struct Time(pub i64); /// Returns a parser for a `len`-digit non-negative number which fits into an i32. @@ -221,7 +222,7 @@ impl fmt::Display for Time { /// A duration specified in 1/90,000ths of a second. /// Durations are typically non-negative, but a `moonfire_db::db::CameraDayValue::duration` may be /// negative. -#[derive(Clone, Copy, Debug, Default, Eq, Ord, PartialEq, PartialOrd)] +#[derive(Clone, Copy, Default, Deserialize, Eq, Ord, PartialEq, PartialOrd, Serialize)] pub struct Duration(pub i64); impl Duration { @@ -230,6 +231,13 @@ impl Duration { } } +impl fmt::Debug for Duration { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + // Write both the raw and display forms. + write!(f, "{} /* {} */", self.0, self) + } +} + impl fmt::Display for Duration { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { let mut seconds = self.0 / TIME_UNITS_PER_SEC; diff --git a/server/src/json.rs b/server/src/json.rs index 6db8429..f87efb8 100644 --- a/server/src/json.rs +++ b/server/src/json.rs @@ -2,6 +2,7 @@ // Copyright (C) 2020 The Moonfire NVR Authors; see AUTHORS and LICENSE.txt. // SPDX-License-Identifier: GPL-v3.0-or-later WITH GPL-3.0-linking-exception. +use base::time::{Duration, Time}; use db::auth::SessionHash; use failure::{format_err, Error}; use serde::ser::{Error as _, SerializeMap, SerializeSeq, Serializer}; @@ -77,9 +78,9 @@ pub struct CameraConfig<'a> { #[serde(rename_all = "camelCase")] pub struct Stream<'a> { pub retain_bytes: i64, - pub min_start_time_90k: Option, - pub max_end_time_90k: Option, - pub total_duration_90k: i64, + pub min_start_time_90k: Option