Delete frame_locked_markers when reusing marker

[sloretz]

Fixes #906

This makes sure to delete the marker from frame_locked_markers_ when reusing an old marker.

If a existing Marker is frame locked, but a new marker uses the same ID and is not frame locked then the marker in markers_ gets deleted, but the frame_locked_markers_ entry was not. That was causing RViz to try to update markers that didn't exist.

Separately, keeping separate std::set for markers_with_expiration_ and frame_locked_markers_ seems like it's keeping state in two places. It might be worth refactoring this code in the future to iterate over markers_ instead of trying to keep three containers in sync.

This PR is backportable

[sloretz]

CI (build: --packages-above-and-dependencies rviz_default_plugins test: --packages-above rviz_default_plugins)

• Linux
• Linux-aarch64
• Windows

[clalancette]

Separately, keeping separate std::set for markers_with_expiration_ and frame_locked_markers_ seems like it's keeping state in two places. It might be worth refactoring this code in the future to iterate over markers_ instead of trying to keep three containers in sync.

While I like that idea in theory, I'm somewhat concerned about the performance implications of doing so. That is, one refactoring I could see would be to add a simple boolean flag to each marker to determine whether it is expired or frame locked. Then we would just have the markers_ map that contains MarkerID -> MarkerBasePtr. That would definitely make the code easier to follow, and prevent bugs like the one you are fixing here. But it would also mean that in order to find these markers, we would have to traverse the entire map to find them, which could be slow with a lot of markers in play. So I'm not sure that it is a straight win (I guess some benchmarking would have to be done).

Regardless, that is not what you are doing here, so I'll just review this code :).

[sloretz]

@Mergifyio backport humble galactic foxy

[mergify]

backport humble galactic foxy

✅ Backports have been created

• #910 Delete frame_locked_markers when reusing marker (backport #907) has been created for branch humble
• #911 Delete frame_locked_markers when reusing marker (backport #907) has been created for branch galactic
• #912 Delete frame_locked_markers when reusing marker (backport #907) has been created for branch foxy