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

Add Azure Device Update code to main #2375

Merged
merged 38 commits into from
Nov 11, 2022
Merged

Add Azure Device Update code to main #2375

merged 38 commits into from
Nov 11, 2022

Conversation

danewalton
Copy link
Member

No description provided.

danewalton and others added 17 commits August 1, 2022 14:20
* 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 <vhvb1989@gmail.com>
* ignore delta update fields from adu and add test with these fields

* clang-format

* pr comments
@ahsonkhan ahsonkhan added this to the 2022-11 milestone Nov 9, 2022
@ahsonkhan ahsonkhan added the IoT label Nov 9, 2022
@danewalton danewalton mentioned this pull request Nov 10, 2022
@ahsonkhan
Copy link
Member

/azp run c - client - tests

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Member

@ahsonkhan ahsonkhan left a 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!

@ahsonkhan
Copy link
Member

ahsonkhan commented Nov 11, 2022

The line coverage for ADU is pretty great. For branch coverage though, other IoT code has set a much higher bar (80%+) :)
image

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:
https://dev.azure.com/azure-sdk/public/_build/results?buildId=1980826&view=codecoverage-tab
image
image

There are a couple places that could benefit from unit tests though, unless those branches are unreachable:
image

@danewalton
Copy link
Member Author

@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.

@danewalton danewalton merged commit bc4c1ee into main Nov 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants