Skip to content

feat: append the option whether to add c8y_IsDevice to child devices #3560

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Apr 15, 2025
Merged

feat: append the option whether to add c8y_IsDevice to child devices #3560

merged 1 commit into from
Apr 15, 2025

Conversation

rina23q
Copy link
Member

@rina23q rina23q commented Apr 11, 2025 •
edited by reubenmiller
Loading

Proposed changes

The static SmartREST template 101 is extended to accept boolean if a newly created child device should hold the c8y_IsDevice fragment.

This change adds a new tedge configuration to control it.

todos:

  • Get the consensus for the name of the new tedge config. => c8y.smartrest.child_device.create_with_device_marker
  • Add (or modify if needed) a system test -> eu-latest is not supporting this new feature yet anyway

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

#3487

Checklist

  • I have read the CONTRIBUTING doc
  • I have signed the CLA (in all commits with git commit -s. You can activate automatic signing by running just prepare-dev once)
  • I ran just format as mentioned in CODING_GUIDELINES
  • I used just check 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

@rina23q rina23q temporarily deployed to Test Pull Request April 11, 2025 16:45 — with GitHub Actions Inactive
@codecov Codecov
Copy link

codecov bot commented Apr 11, 2025

Codecov Report

Attention: Patch coverage is 94.02985% with 4 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...common/tedge_config/src/tedge_toml/tedge_config.rs 66.66% 0 Missing and 2 partials ⚠️
crates/extensions/c8y_mapper_ext/src/config.rs 60.00% 2 Missing ⚠️

📢 Thoughts on this report? Let us know!

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Apr 11, 2025 •
edited
Loading

Robot Results

✅ Passed ❌ Failed ⏭️ Skipped Total Pass % ⏱️ Duration
605 0 3 605 100 1h46m42.236867999s

@rina23q rina23q marked this pull request as ready for review April 14, 2025 09:23
albinsuresh
albinsuresh approved these changes Apr 14, 2025
View reviewed changes
Copy link
Contributor

@albinsuresh albinsuresh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feel free to merge once the name is finalized.

@reubenmiller reubenmiller added the theme:c8y Theme: Cumulocity related topics label Apr 14, 2025
@rina23q rina23q temporarily deployed to Test Pull Request April 15, 2025 10:28 — with GitHub Actions Inactive
albinsuresh
albinsuresh approved these changes Apr 15, 2025
View reviewed changes
Copy link
Contributor

@albinsuresh albinsuresh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

crates/extensions/c8y_mapper_ext/src/config.rs
@@ -62,7 +62,7 @@ pub struct C8yMapperConfig {
pub bridge_service_name: String,
pub bridge_health_topic: Topic,
pub smartrest_use_operation_id: bool,
pub smartrest_child_append_device_fragment: bool,
pub smartrest_child_device_create_with_device_marker: bool,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a request for change: I'm just surprised clippy isn't freaking out with that name 😉.

@reubenmiller reubenmiller added the theme:childdevices Theme: Child device related topics label Apr 15, 2025
reubenmiller
reubenmiller approved these changes Apr 15, 2025
View reviewed changes
Copy link
Contributor

@reubenmiller reubenmiller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved. It worked as expected (when testing it on a tenant that supports the new SmartREST 2.0 template)

@rina23q
feat: append the option whether to add c8y_IsDevice to child devices
36998d3 
The static SmartREST template 101 is extended to accept boolean if a
newly created child device should hold the `c8y_IsDevice` fragment.

This change adds a new tedge configuration to control it.

Signed-off-by: Rina Fujino <rina.fujino.23@gmail.com>
@rina23q rina23q force-pushed the feature/3487/add-is-device-fragment-option-to-smartrest-101-message branch from 70a4a5d to 36998d3 Compare April 15, 2025 15:19
@rina23q rina23q temporarily deployed to Test Pull Request April 15, 2025 15:19 — with GitHub Actions Inactive
@rina23q rina23q added this pull request to the merge queue Apr 15, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 15, 2025
@rina23q rina23q added this pull request to the merge queue Apr 15, 2025
Merged via the queue into thin-edge:main with commit 9f9f0cd Apr 15, 2025
65 of 68 checks passed
@rina23q rina23q deleted the feature/3487/add-is-device-fragment-option-to-smartrest-101-message branch April 16, 2025 09:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@albinsuresh albinsuresh albinsuresh approved these changes

@reubenmiller reubenmiller reubenmiller approved these changes

@didier-wenzek didier-wenzek Awaiting requested review from didier-wenzek didier-wenzek is a code owner

@jarhodes314 jarhodes314 Awaiting requested review from jarhodes314 jarhodes314 is a code owner

Assignees
No one assigned
Labels
theme:c8y Theme: Cumulocity related topics theme:childdevices Theme: Child device related topics
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@rina23q @albinsuresh @reubenmiller