Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Gen 3] Fix warm boot; [Tracker] Cache ESP32 firmware version #2269

Merged
merged 3 commits into from
Jan 20, 2021

Conversation

avtolstoy
Copy link
Member

Problem

  1. We've introduced a regression in Gen3: Network management improvement #2217 and broke warm boot
  2. During handshake with the cloud we need to send ESP32 NCP firmware version as it is one of the module, so it's present in the system describe. On Tracker during the cloud connection establishment it's not guaranteed that ESP32 is on, so we have to turn it on in order to query for its version, which prolongs the handshake process. What complicates things even more is that since Gen3: Network management improvement #2217 we'll also watch the current NCP state closely and power off immediately, so several sequential calls to query ESP32 NCP firmware version may have to power on the ESP32 and initialize it multiple times

Solution

  1. Keep the NCP state as-is unless some command (on, off, up, down) is submitted
  2. Cache ESP32 NCP firmware version in non-volatile memory and read it from there

Steps to Test

  1. Use an app with SEMI_AUTOMATIC modem and suspend the connection process for some time. Wait until the device connects to the cloud, reset with a button, watch the logs - it should reconnect without powering off the NCP
  2. Watch the logs, make sure that ESP32 firmware version is taken out of cache: Cached ESP32 NCP firmware version
  3. Update ESP32 NCP OTA, watch the logs, make sure that ESP32 firmware version changes and gets taken out of cache.

For step 3, the following 0.0.8 ESP32 NCP firmware can be used: tracker-esp32-ncp@0.0.8.zip

Example App

N/A

References

  • [CH71350]

Completeness

  • User is totes amazing for contributing!
  • Contributor has signed CLA (Info here)
  • Problem and Solution clearly stated
  • Run unit/integration/application tests on device
  • Added documentation
  • Added to CHANGELOG.md after merging (add links to docs and issues)


// Just straight up proxying to TlvFile
// TODO: add support for multiple values for a key?
int get(SystemCacheKey key, void* value, size_t length);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: const?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't do const as TlvFile::get() modifies the internal state and cannot be const either, unless we make some member variables mutable.

}
uint16_t version = 0;
// Defaults to zero in case of failure
getWifiNcpFirmwareVersion(&version);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about do this in the Esp32NcpNetif::loop before entering the while loop like we do to the Argon to fetch the MAC address? Just to be consistence to initialize things in one place.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method will get called multiple times throughout the device runtime, so this is the appropriate place to look into cache first and then fallback to calling into ESP32 itself to get the version (and update the cache).

If we are thinking of checking/updating the cache on early boot as we do with a MAC address on an Argon, this is disabled on Tracker already, so that we don't unnecessarily boot ESP32 every time we boot Device OS. Eventually using the caching that was implemented here we'll do the same for MAC address. So, I'd prefer to keep things on as-needed basis.

@avtolstoy avtolstoy force-pushed the feature/tracker-cache-esp32-firmware-version branch from 34245d0 to 32a675d Compare January 20, 2021 14:22
@avtolstoy avtolstoy merged commit 6ce0c7f into develop Jan 20, 2021
@avtolstoy avtolstoy deleted the feature/tracker-cache-esp32-firmware-version branch January 20, 2021 15:26
@avtolstoy avtolstoy modified the milestones: 3.0.0, 2.0.2 Jan 21, 2021
@avtolstoy avtolstoy mentioned this pull request Jun 14, 2021
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants