search

thin-edge / thin-edge.io

The open edge framework for lightweight IoT devices
https://thin-edge.io
Apache License 2.0
210 stars 54 forks source link

refactor: c8y mapper republishes metadata message with `@id` #2925

Closed rina23q closed 2 weeks ago

rina23q commented 3 weeks ago

Proposed changes

Append @id if not given in entity registration messages.

Remark: I haven't decided how to support the main device before any metadata messages for the main device are sent. Should mapper publish as part of init messages? => I gave it up. Read my comment. https://github.com/thin-edge/thin-edge.io/pull/2925#issuecomment-2155552747

Types of changes

Paste Link to the issue

2923

Checklist

Further comments

codecov[bot] commented 3 weeks ago

Codecov Report

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

Project coverage is 78.2%. Comparing base (f186b3f) to head (e6d5178). Report is 31 commits behind head on main.

Additional details and impacted files | [Files](https://app.codecov.io/gh/thin-edge/thin-edge.io/pull/2925?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=thin-edge) | Coverage Δ | | |---|---|---| | [crates/extensions/c8y\_mapper\_ext/src/tests.rs](https://app.codecov.io/gh/thin-edge/thin-edge.io/pull/2925?src=pr&el=tree&filepath=crates%2Fextensions%2Fc8y_mapper_ext%2Fsrc%2Ftests.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=thin-edge#diff-Y3JhdGVzL2V4dGVuc2lvbnMvYzh5X21hcHBlcl9leHQvc3JjL3Rlc3RzLnJz) | `92.5% <100.0%> (-0.1%)` | :arrow_down: | | [crates/extensions/c8y\_mapper\_ext/src/converter.rs](https://app.codecov.io/gh/thin-edge/thin-edge.io/pull/2925?src=pr&el=tree&filepath=crates%2Fextensions%2Fc8y_mapper_ext%2Fsrc%2Fconverter.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=thin-edge#diff-Y3JhdGVzL2V4dGVuc2lvbnMvYzh5X21hcHBlcl9leHQvc3JjL2NvbnZlcnRlci5ycw==) | `84.1% <96.0%> (+0.4%)` | :arrow_up: | ... and [23 files with indirect coverage changes](https://app.codecov.io/gh/thin-edge/thin-edge.io/pull/2925/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=thin-edge)
albinsuresh commented 3 weeks ago

I haven't decided how to support the main device before any metadata messages for the main device are sent. Should mapper publish as part of init messages?

Yes, that seems like as sensisble place for it.

rina23q commented 3 weeks ago

I haven't decided how to support the main device before any metadata messages for the main device are sent. Should mapper publish as part of init messages?

Yes, that seems like as sensisble place for it.

I found this approach has a problem. If te/device/main// registration message is already retained, init function will overwrite the existing metadata message. It is significantly difficult to know if no message is already retained on the topic. There will be the same problem even if we move publishing te/device/main// registration message to tedge connect c8y.

Now I understood why we didn't publish auto generated te/device/main// metadata... I'm even considering to give it up, since it's not as critical as child devices. In any case, we can know the main device's external ID from tedge config.

rina23q commented 3 weeks ago

By the way I'll modify the commit messages later. I didn't know that using @ ends up with mentioning someone who I didn't intend at all :( If someone wants to know how to escape, this seems helpful... https://stackoverflow.com/questions/38862079/github-commit-message-escape-character. In short, the easiest solution is just to avoid using @ in commit messages.

=> Done

github-actions[bot] commented 3 weeks ago

Robot Results

:white_check_mark: Passed :x: Failed :next_track_button: Skipped Total Pass % :stopwatch: Duration
453 0 3 453 100 1h32m35.441934999s