Correctly map ImageManager list model index to topic vector #46
Conversation
Signed-off-by: Kenji Brameld <kenjibrameld@gmail.com>
There was a problem hiding this comment.
Thanks @Kettenhoax for finding the bug and proposing the fix! I have added a test that confirms the behaviour change. Please tell me if it looks good to you, and we can get it merged and backported!
| @@ -154,7 +154,6 @@ std::optional<ImageTopic> ImageManager::getImageTopic(unsigned index) | |||
| void ImageManager::addImageTopicExplicitly(ImageTopic imageTopic) | |||
| { | |||
| beginResetModel(); | |||
| imageTopics.clear(); | |||
There was a problem hiding this comment.
Has this change intentionally been added to this PR? It seems like it fixes a different issue.
There was a problem hiding this comment.
Yep, this was intentional. With the test I added, I couldn't call addImageTopicExplicitly consecutively because the second call clears the topic added by the first call.
As I didn't see a point in having it there, (and so it actually "adds" the image topic, rather than clearing-then-adding).
Signed-off-by: Kenji Brameld <kenjibrameld@gmail.com>
|
Going to merge once CI passes. |
|
Goingto merge, as build failure seems related to ros-sports/ros_image_to_qimage#23 |
|
@Mergifyio backport galactic humble |
✅ Backports have been created
|
Correctly map ImageManager list model index to topic vector (backport #46)
Correctly map ImageManager list model index to topic vector (backport #46)
Most functions of ImageManager interpret index 0 as unselected and map indices > 0 to the actual topics.
This PR does the same in getImageTopic.
Fixes #45