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

Firmware update and access to internal flash via USB requests #1456

Merged
merged 8 commits into from Jan 19, 2018

Conversation

@sergeuz
Copy link
Member

commented Jan 8, 2018

Problem

This PR is based on feature/usb-requests and introduces the following changes:

  • The firmware now can be updated via USB requests (see START_FIRMWARE_UPDATE, FINISH_FIRMWARE_UPDATE, CANCEL_FIRMWARE_UPDATE and FIRMWARE_UPDATE_DATA requests specifically).

  • The system now exposes its internal storage (firmware modules, factory backup, DCT and EEPROM) in a self-descriptive manner via DESCRIBE_STORAGE, READ_SECTION_DATA, WRITE_SECTION_DATA and CLEAR_SECTION_DATA requests.

  • system_ctrl_set_result() now accepts an optional completion callback making it possible for the system to, for example, reset the device only after a reply to the host is sent completely. This helps to avoid USB errors at the client side.

  • system_ctrl_alloc_reply_data() now acts as realloc() and, consequently, system_ctrl_free_reply_data() has been removed.

  • The LISTENING_MODE request has been split into two requests: START_LISTENING and STOP_LISTENING.

  • Protocol files have been moved to a git submodule (firmware-protobuf). The firmware repo still contains the protocol files in a precompiled form, so, normally, there's no need for our users or CI to perform any additional steps in order to build the firmware.

Steps to Test

As of now, the easiest way to test the USB requests is to use the particle-usb library

References

  • [CH10193]
  • [CH10383]

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)

@sergeuz sergeuz added the enhancement label Jan 8, 2018

@sergeuz sergeuz added this to the 0.8.0 milestone Jan 8, 2018

@sergeuz sergeuz requested a review from m-mcgowan Jan 8, 2018

Add a request for getting the actual size of a section data; rename s…
…ome of the requests; update submodule refs

@m-mcgowan m-mcgowan modified the milestones: 0.8.0, 0.8.0-rc.2 Jan 18, 2018

@m-mcgowan
Copy link
Contributor

left a comment

Bravo! 👍

extern const module_bounds_t module_bootloader;

// Modular firmware
extern const module_bounds_t module_system_part1;

This comment has been minimized.

Copy link
@m-mcgowan

m-mcgowan Jan 19, 2018

Contributor

we should pull these into an internal HAL header

@@ -226,7 +226,7 @@ class EEPROMEmulation

// Returns number of bytes that can be stored in EEPROM
// The actual capacity is set to 50% of the records that fit in the smallest page
constexpr size_t capacity() const
static constexpr size_t capacity()

This comment has been minimized.

Copy link
@m-mcgowan

m-mcgowan Jan 19, 2018

Contributor

why remove the const declaration?


DYNALIB_FN(BASE_IDX + 14, system, system_pool_alloc, void*(size_t, void*))
DYNALIB_FN(BASE_IDX + 15, system, system_pool_free, void(void*, void*))
DYNALIB_FN(BASE_IDX + 13, system, system_pool_alloc, void*(size_t, void*))

This comment has been minimized.

Copy link
@m-mcgowan

m-mcgowan Jan 19, 2018

Contributor

any reason not to make the other alloc/free pairs function like realloc?

return 0;
}

const Section INTERNAL_STORAGE_SECTIONS[] = {

This comment has been minimized.

Copy link
@m-mcgowan

m-mcgowan Jan 19, 2018

Contributor

Could we add a storage for the entire flash space? It's low level, but might be useful in some situations.

@@ -77,29 +78,36 @@ particle::SystemControl::SystemControl() :
void particle::SystemControl::processRequest(ctrl_request* req, ControlRequestChannel* /* channel */) {
switch (req->type) {
case CTRL_REQUEST_RESET: {
System.reset();
setResult(req, SYSTEM_ERROR_NONE);
setResult(req, SYSTEM_ERROR_NONE, [](int result, void* data) {

This comment has been minimized.

Copy link
@m-mcgowan

m-mcgowan Jan 19, 2018

Contributor

really nice - I realized that before the system would reset before sending the response. This way is much better.

// Allocate the buffer dynamically
req->reply_data = (char*)t_malloc(size);
if (!req->reply_data) {
if (size > 0) {

This comment has been minimized.

Copy link
@m-mcgowan

m-mcgowan Jan 19, 2018

Contributor

are we not using the system pool any more? what was the reason for using it to begin with?

@m-mcgowan m-mcgowan merged commit 6faab0c into feature/usb-requests Jan 19, 2018

0 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
continuous-integration/travis-ci/push The Travis CI build could not complete due to an error
Details

@m-mcgowan m-mcgowan deleted the feature/fw_update_usb_req branch Jan 19, 2018

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