Entity ID reservation, Dune logging, and addition of new IMC messages

[oysteinvolden]

We have used the imc ros bridge to convert different types of ROS messages to IMC messages, and the bridge works very well. However, we have noticed the following issues:

1. Source entity ID on the IMC messages must be hardcoded based on reserved entity IDs on the DUNE side. Is this the desired approach, or do you have another approach to set the correct entity ID?
2. For the DUNE logging task to see the IMC message from the bridge, the IMC destination address must be set to 0xFFFF, i.e., broadcast to all tasks. This is configured in udp_link.cpp (the publish_multicast() and publish() functions). These are set to 0 by default, is there a reason for this?
3. The IMC message definitions have different return types than those auto-generated using the IMC script and those that follow DUNE by default. Consequently, when adding new IMC message definitions that inherit from the base virtual functions, e.g., in Message.hpp, the return types must be manually changed. An example of this is the function: virtual double getValueFP() in Message.hpp. The proper return type of this function should be: virtual fp64_t getValueFP(). There are other examples of this, and adding new messages would be easier if the base definitions were up to date with DUNE and IMC.

Finally, we have made a 'how-to' document for integrating custom ROS messages, i.e., ROS messages that are not part of default packages such as sensor_msgs, into the imc ros bridge. In our case, we show how customized messages from an SBG INS can be added (https://github.com/SBG-Systems/sbg_ros_driver). Is this of any interest for you?

[krisgry]

A small clarification (as well as a bump 😄): the use case mentioned above is to use sensor data from ROS in DUNE, to e.g. avoid having to port a driver/interface from ROS to DUNE. This is different from what seems to be the main use case of the smarc-project (commanding ROS from Neptus), but is also something we believe to be useful for the community. We will gladly help by adding this to the docs

[nilsbore]

Hi @oysteinvolden and @krisgry ! Sorry for the late reply, I'm on parental leave right now! I'm gonna try to answer your questions, but I'm gonna need @KKalem 's help with some of them:

1. @KKalem ?
2. @KKalem ? :)
3. If I understand you correctly, the version we are using (https://github.com/smarc-project/imc_ros_bridge/tree/noetic-devel/external/imc-cxx/IMC) may not be optimal? Are you suggesting to use the ones from the DUNE repo (https://github.com/LSTS/dune/tree/master/src/DUNE) instead?

The use case sounds perfectly reasonable and we would be happy to accept contributions, the more the merrier!

[oysteinvolden]

Regarding question 3: Yes, we suggest using this one as it is up to date with DUNE.

As @krisgry says, we are happy to contribute :)

[krisgry]

@nilsbore / @KKalem regarding 3; I think it would be beneficial if you could explain why you use the IMC-version you do. Do you primarily use DUNE provided by oceanscan? If that is the case, then I guess the root cause here is that LSTS and Oceanscan DUNE/IMC differs, which is unfortunate and not ideal to handle in this project (?). A similar issue will appear if you implement new IMC-messages, so I think it would make sense to have this project less tied to a specific IMC version (or at least have instructions on how to update/change the IMC messages)

[nilsbore]

@krisgry The only reason we're using this IMC version is because there was a self-contained GitHub repo that we could include within this repo and update easily. If we are to update our IMC version, the main consideration is that it should not include any extra build steps, and that it should be compatible with the latest Neptus versions (the DUNE version should fulfill this last criteria). Happy to hear that you're willing to contribute, you clearly know more about the IMC protocol than we do :)

[KKalem]

Source entity ID on the IMC messages must be hardcoded based on reserved entity IDs on the DUNE side. Is this the desired approach, or do you have another approach to set the correct entity ID?

We actually take the entity IDs as rosparams on launch, unless I am not understanding which IDs you are referring to:

imc_ros_bridge/src/bridge_node.cpp

Lines 62 to 63 in 977fa32

	ros::param::param<int>("~imc_id", imc_src, 30);
	ros::param::param<int>("~imc_src", imc_id, 5);

So you should be able to set them to whatever you have them set as on the DUNE side. We made this like so because our use case uses one bridge per vehicle, so each vehicle sets different IDs for their own bridges. If you want to have one bridge for multiple vehicles/objects, that might require some extensive work.

For the DUNE logging task to see the IMC message from the bridge, the IMC destination address must be set to 0xFFFF, i.e., broadcast to all tasks. This is configured in udp_link.cpp (the publish_multicast() and publish() functions). These are set to 0 by default, is there a reason for this?

I do not remember a particular reason for setting those to 0. Since we are not using DUNE, we had no idea about particular address requirements. Feel free to make it into a parameter, I'm sure there will be other uses eventually :)

[oysteinvolden]

So you should be able to set them to whatever you have them set as on the DUNE side. We made this like so because our use case uses one bridge per vehicle, so each vehicle sets different IDs for their own bridges. If you want to have one bridge for multiple vehicles/objects, that might require some extensive work

Thanks for the reply. This is as we thought :) We are not interested in using a bridge for multiple vehicles. We were just wondering if this was the way it was supposed to be done, and we will continue using this approach then.

I do not remember a particular reason for setting those to 0. Since we are not using DUNE, we had no idea about particular address requirements. Feel free to make it into a parameter, I'm sure there will be other uses eventually :)

Good to know. We may look into making this a parameter such that it can be changed more easily :)