iternio / ovms-link

14 stars 6 forks source link

Telemetry will not be sent more than every 10 seconds #19

Open kezarjg opened 1 year ago

kezarjg commented 1 year ago

The script currently runs sendTelemetryIfNecessary on the events ticker.10, vehicle,on, and vehicle.off. As a result, telemetry will not be sent more frequently than every 10 seconds.

dteirney commented 1 year ago

Only sending at most every 10 seconds was a deliberate choice to avoid chewing through mobile data for little additional value in terms of the experience with ABRP while driving using that app.

dteirney commented 1 year ago

With the implementation now using HTTPS I'm not sure the dog slow TLS implementation in Duktape would support anything faster than a request every 10 seconds.

kezarjg commented 1 year ago

Only sending at most every 10 seconds was a deliberate choice to avoid chewing through mobile data for little additional value in terms of the experience with ABRP while driving using that app.

The variable maxCalibrationTimeout is set to 5 seconds, which implies that telemetry could be sent every 5 seconds. However, the actual behavior of the code limits telemetry sending to at most every 10 seconds.

kezarjg commented 1 year ago

With the implementation now using HTTPS I'm not sure the dog slow TLS implementation in Duktape would support anything faster than a request every 10 seconds.

I'm using 5 seconds in my fork, and it's working well. There are very few instances of the HTTP request calls that are taking more than 1 second.

dteirney commented 1 year ago

The intention is still one payload at most every 10 seconds, regardless.

maxCalibrationTimeout was set to be half the ticker interval to help ensure that it would be sent, as the send determination is based on the time that the last metrics collection was retrieved (in the utc prop) and the point in time instant just before the decision to send or not. There's more than zero time consumed in the processing of each tick. Half the tick interval is a common model if the intention is to have the content very likely to be sent within the current tick and not fall over to the next tick due to processing time within the tick.

dteirney commented 1 year ago

I just refreshed my memory; the difference between each collected metrics utc timestamp was used rather than when the payload was last sent (IIRC, that was the previous model). With a single call to OvmsMetrics.GetValues, that was more reliable than using the timestamp for the last send. With some other changes sitting in PRs to chop up the metrics collections into dozens of requests across the time domain, the utc field no longer represents the instant in which the metrics were captured.