Open albinsuresh opened 11 months ago
There also seems to be an important difference in behaviour between publish_topic_from_ancestors
and C8yTopic::to_topic
for ChildSmartRestResponse(String)
variant:
publish_topic_from_ancestors
uses entity's ancestors, so when a child is nested, it creates a topic that contains all devices in the hierarchy:
https://github.com/thin-edge/thin-edge.io/blob/e4fdb007ebd3ca6848214187e84897a623e55f33/crates/core/c8y_api/src/smartrest/topic.rs#L88-L92
C8yTopic::to_topic
doesn't take nesting into account, so smartrest publish topics it generates for nested child devices may be wrong
https://github.com/thin-edge/thin-edge.io/blob/e4fdb007ebd3ca6848214187e84897a623e55f33/crates/core/c8y_api/src/smartrest/topic.rs#L19-L55
C8yTopic::to_topic
is used by the availability actor, so is it possible it may not work correctly for nested child devices?
EDIT: There seem to be some checks specifically for nested child devices (58b1ebb), in which case perhaps there's no problems but it's still a bit confusing why using a "wrong" smartrest publish topic is fine.
Is your refactoring request related to a problem? Please describe.
The
c8y_api::smartrest::topic
has various mechanisms to generate SmartREST topics for the main device, immediate child device, nested child devices and services. The recently introducedpublish_topic_from_ancestors
API already covers most of these cases and the rest are just redundant, especially the ones likeC8yTopic::ChildSmartRestResponse
, dedicated for immediate child devices only.Describe the solution you'd like
Refactor the
c8y_api::smartrest::topic
module with a smaller set of helper functions to create topics for all kinds of targets: the main device, immediate child device, nested child devices and services.