search

golioth / reference-design-template

Template for making new Golioth Reference Design repositories
Apache License 2.0
17 stars 5 forks source link

app_sensors: only stream sensor data if connected #108

Closed hasheddan closed 1 month ago

hasheddan commented 1 month ago

When using LTE, we may start fetching sensor readings before a connection is established with Golioth. Readings can be shown on a local display during this period, but attempting to send to Golioth will fail. This updates the sensor reading streaming, which is just a counter in this case, to only attempt to stream readings to Golioth when connected. This mirrors the behavior of the battery monitor.

(ref for rationale on not blocking until connected: https://github.com/golioth/reference-design-template/pull/72/files#r1489688622)

szczys commented 1 month ago

This was by design as the failure indicates that you do not yet have a connection to Golioth.

With this update, the app will show it is taking sensor readings, but they will not appear at the destination. Do you think an else clause that prints a message about skipping the push until a connection is available would help with understanding the current state?

szczys commented 1 month ago

I do think it would be useful to have a message saying something like "No connection available, data not sent to Golioth". It's easy to miss the message that says a connection could take awhile and a recurring reminder that there is no connection is very helpful for new folks (and me!) troubleshooting their prototypes.

I'm fine with updating the battery monitor too. It currently only works on the aludel-mini which is deprecated so I don't think we need to do a lot of work there until we add aludel-elixir support for reading battery levels.

hasheddan commented 1 month ago

ack! updated -- example logging:

*** Booting My Application v2.4.0-1010dc0b83df ***
*** Using nRF Connect SDK v2.7.0-5cb85570ca43 ***
*** Using Zephyr OS v3.6.99-100befc70c74 ***
[00:00:00.640,380] <dbg> golioth_rd_template: main: Start Reference Design Template sample
[00:00:00.640,411] <inf> golioth_rd_template: Firmware version: 2.4.0
[00:00:00.647,033] <inf> golioth_rd_template: Modem firmware version: mfw_nrf9160_1.3.6
[00:00:00.647,064] <inf> golioth_rd_template: Connecting to LTE, this may take some time...
[00:00:00.693,054] <dbg> app_sensors: app_sensors_read_and_stream: No connection available, skipping streaming counter: 0
[00:01:00.693,115] <dbg> app_sensors: app_sensors_read_and_stream: No connection available, skipping streaming counter: 1
[00:01:59.535,980] <inf> lte_monitor: Network: Searching
[00:02:00.693,237] <dbg> app_sensors: app_sensors_read_and_stream: No connection available, skipping streaming counter: 2
[00:03:00.693,359] <dbg> app_sensors: app_sensors_read_and_stream: No connection available, skipping streaming counter: 3
hasheddan commented 1 month ago

The change looks good, but would you be able to separate the formatting changes into their own commit before applying the actual change?

@szczys will do 👍🏻