-
Notifications
You must be signed in to change notification settings - Fork 56
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
Child device firmware management specification #1998 #2007
Child device firmware management specification #1998 #2007
Conversation
2505253
to
31913f9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just some minor command and improvements
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel there are two parts in this doc, one is about How to use the firmware plugin
, and the second one is more details about the whole process. So, I feel it's better to move the first part to a how to
doc and the second part can remain here. Also, How to doc
can give a link to the second part for more info.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for writing the specification finally!
Is missing
- Content of operation files under
$TEDGE_DATA_PATH/firmware/<op_id>
.
In addition, I commented about some mismatched with current implementation.
Child Device Connector ->> C8Y Firmware Plugin: MQTT: firmware_update response with operation status: "successful" | ||
|
||
C8Y Firmware Plugin ->> C8Y Cloud: MQTT: Update c8y_Firmware operation status to "SUCCESSFUL" | ||
C8Y Firmware Plugin ->> C8Y Firmware Plugin: Remove the symlink from file-transfer repository but keep the cached firmware copy for reuse |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it removes symlink... instead deleting a firmware info file under FIRMWARE_OP_PATH
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should be removing those symlinks as it makes it easier for the device admin to cleanup the firmware cache, when needed by just looking at the files that don't have any symlinks pointed to it.
* `reason`: The reason for the failure, applicable only for `failed` status. | ||
1. On reception of an operation status message, the plugin maps it to SmartREST and forwards it to the cloud. | ||
* When a `success` or `failed` status message is finally received, | ||
then the plugin cleans up the corresponding operation file at `$FIRMWARE_OP_PATH/$OP_ID` and |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I said above, the current plugin implementation doesn't clean up symlink. If we need it, we need to change the code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need to change the code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So a ticket needs to be created when this PR is merged.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done: #2023
24a9a18
to
12bebcf
Compare
It was intentional as this is a reference guide that must also define how the installation of the plugin and configuration of the plugin are also supported. The dev of the plugin should have used this doc to write all that installation and configuration code as well.
Done |
Added the details |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved. With one suggestion to improve the introduction.
|
||
1. The plugin, on recept of a `c8y_Firmware` request from Cumulocity for a child device named `$CHILD_DEVICE_ID` | ||
in the format `515,$CHILD_DEVICE_ID,$FIRMWARE_NAME,$FIRMWARE_VERSION,$FIRMWARE_URL` | ||
1. Validate if the same firmware update operation is already in progress |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[This is not a change request, just a piece of advice for future docs].
A nice way to ease reading such prose, is to focus on the happy/simple path and to address exceptions/corner-cases as complements. For instance,
thin-edge
checks the request is new and the firmware not in cache.- It downloads the firmware using the sha-256 digest as file name.
- It creates a file named after the operation id.
- ...
Then the alternative paths:
- If the operation is already in progress, then ...
- If the firmware is in cache ...
Robot Results
Passed Tests
|
Proposed changes
Specification for
c8y-firmware-plugin
.Types of changes
Paste Link to the issue
Checklist
cargo fmt
as mentioned in CODING_GUIDELINEScargo clippy
as mentioned in CODING_GUIDELINESFurther comments