ika-rwth-aachen / etsi_its_messages

ROS / ROS 2 Support for ETSI ITS Messages for V2X Communication
MIT License
43 stars 8 forks source link

CLASS support #13

Closed v0-e closed 3 months ago

v0-e commented 6 months ago

Hey guys, Let me start by thanking you for the great work on the whole ROS ecosystem and also this repo specifically, the community needs better ASN.1 <-> ROS tools like this one :)

On the issue, do you have any plans to support CLASS since it is included in recent ITS message versions like the last CPMs? As you may know, asn1tools unfortunately does not support CLASS nor do I think there is a planned effort to do it.

There is also a seemingly inactive work by the @virtual-vehicle team which uses asn1c to parse the ASN.1 files and then create the ROS messages, missing only the headers-based conversion. Maybe something similar could be worth exploring.

Thanks!

jpbusch commented 6 months ago

Hi @v0-e,

Nice to read that you like our repo and that you are interested in improving it. The issue you described is already known to us, but we don't have a concrete plan to solve it yet. If you are interested in helping us with this and also contributing to this repo, we would be happy to exchange ideas and work together to find the best solution.

We look forward to hearing from you!

lreiher commented 6 months ago

Current idea is to implement two backends (.msg, conversion headers) with https://github.com/librasn/rasn, a Rust library for parsing ASN.1.

v0-e commented 6 months ago

Until a stable version is reached and a nice way to integrate the new compilers/backends in this repo is figured out, development can be tracked here.

v0-e commented 5 months ago

An update, The roundtrip conversion for CAMs and DENMs seems to be working. Tested using the Converter/etsi_its_conversion app. I'm currently working on the conversion for CPMv2 (with CLASS).

v0-e commented 5 months ago

CPMv2 conversion also seems to be working. Keep in mind though that the testing was not comprehensive, i.e., not all fields were tested as there are a lot, though the CLASS-related fields are ok.

On the integration of the new code generation tools with this repo, I have some comments/questions,

  1. Do you prefer running the code gen containerized like it is done with asn1c? Compiling the new code generators requires Rust to be installed;
  2. Some complications may arise from the use of ETSI ITS CDD v1.3.1 and CDD v2.2.1 simultaneously, as some types with the same name are defined in both dictionaries. ROS also seems to have problems resolving stuff like StationID (v1.3.1) and StationId (v2.2.1). I fear that using different message versions in the same binary may result in some linking issues, in both sides: on the asn1c generated code and ROS generated code. For example for the testing, I've modified the conversion package C++ code to only convert CPMv2, removing all the includes to CAM/DENM. A future solution for this maybe adding a prefix associated with the CDD version to all generated types;
  3. I'll make a PR keeping the current Python-based code generation, adding the new tools as an option inside the utils/ dir. I'll then continue the development of the new code gen from the standalone repo to this one;
lreiher commented 5 months ago

Cool, sounds like quick progress, looking forward to testing it soon!

Regarding your questions:

  1. Yes, having the code generation containerized definitely simplifies the step. I suggest to use a similar approach as with the current code generation, store a Dockerfile and have dedicated scripts that start the container with the right arguments.
  2. On the ROS side, there should be no mismatches due to the namespaces, right? etsi_its_cam_msgs::msg::StationId is different from etsi_its_denm_msgs::msg::StationId. You are right though that the current implementation could lead to issues on the C-generated code side. Could it be an option to also add namespaces to those? Note that we already post-process the asn1c-generated files in asn1ToC.py. In my opinion, having dedicated CDDs for each message type is a good way of supporting different versions. Perhaps we could then also have conversion functions between CDDs of the same version, e.g., etsi_its_cam_coding::StationID to etsi_its_denm_coding::StationID. What do you think?
  3. Sounds good, we are happy to test your implementation as soon as possible. While the PR is open, we can then discuss how and in what state we are going to merge it, either in parallel or as a replacement to the current code generation.
lreiher commented 3 months ago

Closed with https://github.com/ika-rwth-aachen/etsi_its_messages/pull/17 and https://github.com/ika-rwth-aachen/etsi_its_messages/pull/20, CPM is here! 🚀