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

Feature/immediate updates #1732

Merged
merged 16 commits into from Apr 12, 2019

Conversation

@m-mcgowan
Copy link
Contributor

commented Mar 19, 2019

Problem

The original implementation of System.disableUpdates() did not operate in tandem with the cloud, and did not work in an intuitive manner as a consequence.

Solution

System events from the cloud are used to notify the device that updates are pending and to force updates enabled when previously disabled in firmware.

The device publishes system events in response to firmware updates being enabled or forced.
The device listens to system events and updates the updates pending and forced flags accordingly.

The CoAP response when updates are disabled has been changed to 5.03 (Service Unavailable) and the response carried with a token, so the cloud reacts immediately.

System events were previously being handled on the application thread. This has been changed to handle them on the system thread.

Steps to Test

The test application in user/tests/app/immediate_updates. The SDD describes the manual test plan.

References

  • [CH24823] - Immediate firmware updates epic
  • [CH27381]

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)

@m-mcgowan m-mcgowan requested a review from sergeuz Mar 19, 2019

@m-mcgowan m-mcgowan added this to the 1.2.0-rc.1 milestone Mar 21, 2019

m-mcgowan added 14 commits Mar 19, 2019
sends the Service Unavailable response (5.03) when the OTA cannot be …
…performed. [CH15025]. This also fixes a bug where the token was not being set, and so the response was not recognized as an answer to the UpdateBegin request.
processes system events on the system even thread, rather than unnece…
…ssarily marshaling them to the application thread.
removes unused `cloud_connected_millis` adds headers required for gcc…
… build and fixes up the Communications Catch tests build, which is mostly about including the `crypto` and `third_party`mbedtls` modules.

@m-mcgowan m-mcgowan force-pushed the feature/immediate_updates branch from 6af7339 to 52a8ece Apr 4, 2019

@m-mcgowan m-mcgowan changed the base branch from develop to release/v1.2.0 Apr 4, 2019

@sergeuz
sergeuz approved these changes Apr 4, 2019
Copy link
Member

left a comment

I left some nitpicking comments, but the code looks good. Thanks for fixing the comms tests!

communication/src/chunked_transfer.cpp Show resolved Hide resolved
communication/src/dtls_session_persist.h Outdated Show resolved Hide resolved
@@ -292,11 +292,17 @@ void invokeEventHandlerString(uint16_t handlerInfoSize, FilteringEventHandler* h
invokeEventHandlerInternal(handlerInfoSize, handlerInfo, name.c_str(), data.c_str(), reserved);
}

void SystemEvents(const char* name, const char* data);

This comment has been minimized.

Copy link
@sergeuz

sergeuz Apr 4, 2019

Member

Interestingly, this function seems to shadow the SystemEvents enum without causing a compilation error

system/src/system_cloud_internal.cpp Show resolved Hide resolved
system/src/system_cloud_internal.cpp Outdated Show resolved Hide resolved
system/src/system_cloud_internal.cpp Outdated Show resolved Hide resolved
system/src/system_update.cpp Outdated Show resolved Hide resolved
system/src/system_update.cpp Outdated Show resolved Hide resolved

@m-mcgowan m-mcgowan merged commit 3ed2999 into release/v1.2.0 Apr 12, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@m-mcgowan m-mcgowan deleted the feature/immediate_updates branch Apr 12, 2019

@sergeuz sergeuz removed the do not merge label May 16, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.