fix: Capability message for firmware_update workflow by albinsuresh · Pull Request #3146 · thin-edge/thin-edge.io

albinsuresh · 2024-09-30T15:14:27Z

Proposed changes

Tedge-agent to publish empty capability message when a workflow is defined for firmware_update operation.

Types of changes

Bugfix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Improvement (general improvements like code refactoring that doesn't explicitly fix a bug or add any new functionality)
Documentation Update (if none of the other choices apply)
Breaking change (fix or feature that would cause existing functionality to not work as expected)

Paste Link to the issue

#3145

Checklist

I have read the CONTRIBUTING doc
I have signed the CLA (in all commits with git commit -s)
I ran cargo fmt as mentioned in CODING_GUIDELINES
I used cargo clippy as mentioned in CODING_GUIDELINES
I have added tests that prove my fix is effective or that my feature works
I have added necessary documentation (if appropriate)

Further comments

codecov · 2024-09-30T15:26:14Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Additional details and impacted files

📢 Thoughts on this report? Let us know!

github-actions · 2024-09-30T16:10:18Z

Robot Results

✅ Passed	❌ Failed	⏭️ Skipped	Total	Pass %	⏱️ Duration
505	0	2	505	100	1h36m23.646114s

reubenmiller · 2024-10-01T05:28:32Z

tests/RobotFramework/tests/cumulocity/firmware/firmware_operation_workflow.robot

I think we should just replace the existing system tests in this file: tests/RobotFramework/tests/cumulocity/firmware/firmware_operation.robot, as the old test file was created before thin-edge.io had official support for c8y_Firmware, and now the existing test add no value.

Okay. I left it as-is since it's still testing the "C8Y operation overriding" mechanism with the c8y_Firmware operation definition file. If we don't really need that anymore, then yeah I can replace that with this one.

Since we have workflows, we don't need the older operation declaration, especially for the c8y_Firmware operation as the official recommended extension point is to create a workflow.

albinsuresh · 2024-10-01T05:56:45Z

tests/RobotFramework/tests/cumulocity/firmware/firmware_operation_workflow.robot

+    Set Suite Variable    $DEVICE_SN
+    Device Should Exist    ${DEVICE_SN}
+
+    Execute Command    tedge config set c8y.enable.firmware_update true


Wondering why we've disabled this by default at the mapper level. Unless someone explicitly creates a C8Y operation file or defines a workflow, the supported operation won't be declared by the mapper. So, I'm not sure why we want to disable the whole mapping itself, by default. @rina23q do you remember why? Can we switch to back to true?

I believe it was done to ensure we don't break systems which are using c8y-firmware-plugin...though we do need to think about a cleaner upgrade path (though correct me if I'm wrong here).

I think first mapper implementation was done while agent side was not completed. That's why it was marked false as a temporary solution, however, it's forgotten to be true, even after firmware implementation in agent side was done...

Resolved by bdaabc5

reubenmiller · 2024-10-01T06:41:56Z

tests/RobotFramework/tests/cumulocity/firmware/firmware_operation_workflow.robot

+
+*** Test Cases ***
+Send firmware update operation from Cumulocity IoT
+    Cumulocity.Should Contain Supported Operations    c8y_Firmware


It might also be worthwhile to add a check for the local capability message as well, e.g. te/device/main///cmd/firmware_update, to ensure that the functionality on the main device is discoverable to other clients listening to the same MQTT broker.

Resolved by bdaabc5

reubenmiller · 2024-10-01T12:27:31Z

tests/RobotFramework/tests/cumulocity/firmware/firmware_operation.robot

 Test Teardown       Get Logs

-Test Tags           theme:c8y    theme:firmware    theme:plugins
+Test Tags           theme:tedge_agent


We can still have the c8y theme on this as we are also testing the c8y_Firmware cloud operation, and tedge-agent is not needed as we would have to add the tag to almost all the non-cloud features

Suggested change

Test Tags theme:tedge_agent

Test Tags theme:c8y theme:firmware

reubenmiller · 2024-10-01T12:29:46Z

tests/RobotFramework/tests/cumulocity/firmware/firmware_operation.robot

+Library             Cumulocity

-Test Setup          Custom Setup
+Suite Setup         Custom Setup


Why do we need to change to a Suite Setup, it would be better to leave it untouched as Test Suite is less prone to unexpected side effects from preceding test cases (in the same file).

Suggested change

Suite Setup Custom Setup

Test Setup Custom Setup

Resolved by d2c2cff

reubenmiller · 2024-10-01T12:30:45Z

tests/RobotFramework/tests/cumulocity/firmware/firmware_operation.robot

 Resource            ../../../resources/common.resource
-Library             Cumulocity
 Library             ThinEdgeIO
+Library             Cumulocity


Revert the Libary import sequence (not change is necessary)

Library Cumulocity Library ThinEdgeIO

Resolved by d2c2cff

didier-wenzek

Approved (rust updates)

reubenmiller

Looks good (Tests and change of default firmware_update setting)

albinsuresh temporarily deployed to Test Pull Request September 30, 2024 15:14 — with GitHub Actions Inactive

albinsuresh had a problem deploying to Test Auto September 30, 2024 15:53 — with GitHub Actions Failure

reubenmiller reviewed Oct 1, 2024

View reviewed changes

albinsuresh force-pushed the fix/3145/capability_message_for_firmware_update_workflow branch from e5be66b to 57cdbcb Compare October 1, 2024 05:43

albinsuresh temporarily deployed to Test Pull Request October 1, 2024 05:43 — with GitHub Actions Inactive

albinsuresh temporarily deployed to Test Auto October 1, 2024 05:48 — with GitHub Actions Inactive

albinsuresh commented Oct 1, 2024

View reviewed changes

albinsuresh marked this pull request as ready for review October 1, 2024 06:05

albinsuresh requested review from a team, didier-wenzek, gligorisaev, jarhodes314 and rina23q as code owners October 1, 2024 06:05

reubenmiller reviewed Oct 1, 2024

View reviewed changes

reubenmiller added theme:firmware Theme: Firmware update topics theme:workflows Theme: Workflow engine topics labels Oct 1, 2024

albinsuresh force-pushed the fix/3145/capability_message_for_firmware_update_workflow branch from 57cdbcb to bdaabc5 Compare October 1, 2024 12:11

albinsuresh temporarily deployed to Test Pull Request October 1, 2024 12:11 — with GitHub Actions Inactive

albinsuresh temporarily deployed to Test Auto October 1, 2024 12:16 — with GitHub Actions Inactive

reubenmiller reviewed Oct 1, 2024

View reviewed changes

fix: Capability message for firmware_update workflow

d2c2cff

didier-wenzek approved these changes Oct 1, 2024

View reviewed changes

albinsuresh force-pushed the fix/3145/capability_message_for_firmware_update_workflow branch from bdaabc5 to d2c2cff Compare October 1, 2024 12:39

albinsuresh temporarily deployed to Test Pull Request October 1, 2024 12:39 — with GitHub Actions Inactive

albinsuresh requested a review from reubenmiller October 1, 2024 12:40

reubenmiller approved these changes Oct 1, 2024

View reviewed changes

albinsuresh temporarily deployed to Test Auto October 1, 2024 12:57 — with GitHub Actions Inactive

albinsuresh enabled auto-merge October 1, 2024 13:10

albinsuresh added this pull request to the merge queue Oct 1, 2024

albinsuresh mentioned this pull request Oct 1, 2024

cmd/firmware_update topic is not published when using a firmware_update workflow #3145

Closed

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 1, 2024

rina23q approved these changes Oct 1, 2024

View reviewed changes

albinsuresh added this pull request to the merge queue Oct 1, 2024

Merged via the queue into thin-edge:main with commit 4e0a879 Oct 1, 2024

albinsuresh deleted the fix/3145/capability_message_for_firmware_update_workflow branch October 22, 2024 06:45

	Test Tags theme:tedge_agent
	Test Tags theme:c8y theme:firmware

Conversation

albinsuresh commented Sep 30, 2024 • edited by reubenmiller Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed changes

Types of changes

Paste Link to the issue

Checklist

Further comments

Uh oh!

codecov bot commented Sep 30, 2024

Codecov Report

Uh oh!

github-actions bot commented Sep 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Robot Results

Uh oh!

Choose a reason for hiding this comment

Uh oh!

albinsuresh Oct 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

reubenmiller Oct 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

didier-wenzek left a comment

Choose a reason for hiding this comment

Uh oh!

reubenmiller left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

albinsuresh commented Sep 30, 2024 •

edited by reubenmiller

Loading

github-actions bot commented Sep 30, 2024 •

edited

Loading

albinsuresh Oct 1, 2024 •

edited

Loading

reubenmiller Oct 1, 2024 •

edited

Loading