Open kaarmu opened 1 year ago
It's intentional that mocap is not publishing to /state
. The perspective that was taken here is that the main role of the mocap is to serve as a measurement tool, instead of a supported localization approach, and therefore it felt like publishing to /state
would conflict with this, since one would not be able to run mocap at the same time as indoor localization. It is possible to use the MotionCaptureInterface
directly in a SVEA manager object:
https://github.com/KTH-SML/svea_mocap/blob/61e792e48009a2c5f2ba0ecdc5508bd4545caae6/scripts/localization_comparison.py#L45-L47
For my understanding, why is it important that the mocap data is published to the /state
topic?
I don't think it is important for most cases but it is probably nice to have (maybe?). The first thought I had was if there is something listening to /state
and we want that thing to use the mocap state as /state
, e.g. rosbridge server. But maybe we shouldn't consider that?
I'm not exactly sure why it was implemented for these tests but that's what came to mind when I saw it.
Frank is right, the fact that the mocap publishes on /state
is conflicting with the amcl localization node. Our idea was to use directly the mocap state as a localisation tool, without running amcl at all. This is due to the fact that still the amcl can't be considered as accurate and stable, especially on longer runs. Furthermore, we wanted to rely on precise localisation in order to have higher-level components tested, avoiding additional inaccuracies. We thought it might have been of common interest.
However, this coould have been accomplished also using MotionCaptureInterface as localisation interface in the svea manager object. In this case, rotation and linear offset between map and mocap frames should still be kept into account to have it working correctly on the sml map.
I would propose an alternative that involves some renaming throughout the codebase:
Since the SVEA platform's deployment contexts are becoming more diverse (no longer just floor 2) and we are trying to explore/integrate more types of localization, it might be good if we rename LocalizationInterface
to IndoorLocalizationInterface
or AMCLLocalizationInterface
.
Along this direction, I'd also propose we do a corresponding rename of /state
to /indoor_state
or /amcl_state
.
This way we can move away from having the AMCL based localization as the first-class localization method. This will also fit in with the efforts with adding RTK-based localization to the SVEA platform.
Thoughts?
Hmm, I still like the idea of having one /state
though but I agree that it's probably good to move in this direction.
I like having one /state
as well, but I think we will need to decide what localization method is considered first-class. We might want to shelve this discussion for a couple of weeks and wait until we have more understanding on RTK and wheel encoders before deciding what to set as the first-class localization method.
Okay, let's pause this here and come back to it later :+1:
While @Fedezac and @mich-pest were testing they found that the mocap weren't publishing to
/state
. They then created a publisher for that. I'm not sure if this is supported already or not. Probably @frankjjiang already knows this or has input on how it should be.From our discussion @Fedezac and @mich-pest, could you create a draft PR with the "mocap adapter" for now. Let's then check whether or not it is needed.