wip add timestamp to server owned call#1504
Conversation
eddbbt
force-pushed
the
timestamp_server_own
branch
from
bf4e5c7 to
8b243df
Compare
Signed-off-by: Eddy Babetto <eddy.babetto@secomind.com>
Signed-off-by: Eddy Babetto <eddy.babetto@secomind.com>
eddbbt
force-pushed
the
timestamp_server_own
branch
from
891ad5c to
4646234
Compare
| path, | ||
| value | ||
| value, | ||
| timestamp: timestamp |
There was a problem hiding this comment.
the previous behavior did not set the timestamp, resulting in a map with only the :v key being set
after this change, we will always set the :t key in the value we send to the device
I believe the device only sets the timestamp when the value has explicit timestamp, my gut feeling says we should do the same BUT we can ask the embedded team for their opinion on the matter
| case explicit_timestamp do | ||
| false -> DateTime.utc_now() | ||
| {:ok, timestamp} -> timestamp | ||
| end |
There was a problem hiding this comment.
the fact that this value can be either false (a boolean) or {:ok, timestamp} (a tuple) is quite cursed. I suggest either making it a classic result
case explicit_timestamp do
{:ok, timestamp} -> timestamp
:error -> DateTime.utc_now()
endor just having a nullable datetime
| ) do | ||
| now = DateTime.utc_now() | ||
| # check for explicit timestamp | ||
| now = |
There was a problem hiding this comment.
maybe value_timestamp better reflects the intent, now worked when it was only "now"
|
|
||
| explicit_timestamp = | ||
| with true <- | ||
| Queries.interface_has_explicit_timestamp?(realm_name, interface_row.interface_id), |
There was a problem hiding this comment.
in their respective functions (update_{individual,object}_interface_values), we're already querying for the mappings
I think we can avoid this extra query by just delaying setting the timestamp until after we've read the mappings from the db
2 participants
addedd timestamp to message received, like (it was empty)