-
Notifications
You must be signed in to change notification settings - Fork 55
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
Deprecate tedge topic parsing method #2818
Deprecate tedge topic parsing method #2818
Conversation
Signed-off-by: Didier Wenzek <[email protected]>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files
|
Robot Results
|
let child_id = match topic.split('/').collect::<Vec<&str>>()[..] { | ||
// This plugin is still listening to the deprecated `tedge` command topic | ||
["tedge", child_id, "commands", "res", "firmware_update"] if !child_id.is_empty() => { | ||
child_id.to_string() | ||
} | ||
_ => { | ||
return Err(FirmwareManagementError::InvalidTopicFromChildOperation { | ||
topic: topic.into(), | ||
}) | ||
} | ||
}; |
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.
note: I think this not only removes the deprecated function, but also fixes the wrong behaviour
Previously get_child_id_from_child_topic
just extracted child id, but the topic itself wasn't checked, so it could be anything.
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.
Indeed, this also fixes the behavior and I failed to missed that trying at first to use one of the tedge_api::MqttSchema
method!
Proposed changes
Deprecate
get_child_id_from_child_topic
, a method parsing the now deprecatedtedge
command topic.Types of changes
Paste Link to the issue
Checklist
cargo fmt
as mentioned in CODING_GUIDELINEScargo clippy
as mentioned in CODING_GUIDELINESFurther comments