-
Notifications
You must be signed in to change notification settings - Fork 123
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
Add Azure Device Update code to main #2375
Conversation
* Change ADU client update id to match service specification The update-id reported by the ADU agent, as defined by the ADU service, is an escaped string, and has no need to be split into a separate struct with the individual components (provider, name, version). This change simplifies the ADU agent state generation by following this specification, as well as removes a bug-prone buffer manipulation from az_iot_adu_client.c. * Fix code style * Remove left-over printfs * Address code review comments * Fix spacing
Co-authored-by: Victor Vazquez <[email protected]>
* ignore delta update fields from adu and add test with these fields * clang-format * pr comments
/azp run c - client - tests |
Azure Pipelines successfully started running 1 pipeline(s). |
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.
Didn't dive too deep into each of the implementation details, but overall, looks good to me!
The line coverage for ADU is pretty great. For branch coverage though, other IoT code has set a much higher bar (80%+) :) Consider adding some edge case tests in the future to improve code coverage (file an issue to track it). At a quick glance, many of the uncovered branches aren't functionally relevant at the moment, or are about invalid/incomplete JSON reading/writing: There are a couple places that could benefit from unit tests though, unless those branches are unreachable: |
@ahsonkhan We can definitely look to get some of those numbers up. Branches might be tough because of the complexity of the JSON but we can do the simpler ones for sure. |
No description provided.