Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] SPATEM/MAPEM Support #28

Draft
wants to merge 78 commits into
base: main
Choose a base branch
from
Draft

[WIP] SPATEM/MAPEM Support #28

wants to merge 78 commits into from

Conversation

lreiher
Copy link
Member

@lreiher lreiher commented Jul 10, 2024

Current WIP State

  • coding packages successfully generated by asn1c
  • download of external asn1 definitions is automated
  • generation of msgs and conversion packages is successful, but codegen-rust modifications still need to be verified; also, doesn't build yet

lreiher and others added 30 commits November 6, 2023 00:27
duplicates in its-pdu and dsrc resolved by commenting them in dsrc for now
@lreiher
Copy link
Member Author

lreiher commented Sep 2, 2024

Hey @v0-e,

would you mind taking a look at this branch? I can successfully generate the C-structs for SPATEM/MAPEM, but msgs/conversion generation fails with an error like the following.

thread 'main' panicked at src/msgs/bin.rs:23:35:
called `Result::unwrap()` on an `Err` value: Error { kind: InvalidData, message: "stream did not contain valid UTF-8" }
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

In order to test this yourself, make sure to initialize the submodules recursively and to execute ./asn1/external/download.sh for downloading external ASN.1 dependencies for the is_ts103301 submodule.

Also tagging @jpbusch, FYI.

@james-ward
Copy link

@lreiher I have taken a look at your code and I can help to make it work a little bit more.
First thing is to delete the offending characters in the source .asn file. This will show the lines:

grep -axv '.*' asn1/raw/is_ts103301/build/asn1/ISO14816_AVIAEINumberingAndDataStructures.asn

Then you can run utils/codegen/codegen-rust/asn1ToRosMsg.py ....

This will throw an error because the Rust version of the tool doesn't recognise BITSTRING types.

The Python version of the script seems to work: utils/codegen/codegen-py/asn1ToRosMsg.py ....

HTH - this isn't a full fix, but might help get this PR further. This is a really great project and we'd love to use it too!

@lreiher
Copy link
Member Author

lreiher commented Sep 16, 2024

@james-ward Thank you very much for the hint! I have pushed an automated patch of the offending characters.

I have then tried to implement handling of bit string in the rust codegen and successfully generated messages and conversion headers. However, we will have to look into the bit string handling again to verify. I'm a little confused right now why this problem hasn't yet surfaced with the other message types.

raw_def = asn1_raw[type]
raw_def = asn1_raw[type] if type in asn1_raw else ""
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some raw message type definitions are apparently not extracted correctly, e.g., ConnectedTrajectory.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a circular dependency in the headers - e.g.

--- stderr: etsi_its_mapem_ts_coding                                                                  
In file included from /home/james/zio/src/etsi_its_messages/etsi_its_coding/etsi_its_mapem_ts_coding/include/etsi_its_mapem_ts_coding/mapem_ts_SignalHeadLocation.h:15,
                 from /home/james/zio/src/etsi_its_messages/etsi_its_coding/etsi_its_mapem_ts_coding/include/etsi_its_mapem_ts_coding/mapem_ts_SignalHeadLocationList.h:44,
                 from /home/james/zio/src/etsi_its_messages/etsi_its_coding/etsi_its_mapem_ts_coding/include/etsi_its_mapem_ts_coding/mapem_ts_MapData-addGrpC.h:46,
                 from /home/james/zio/src/etsi_its_messages/etsi_its_coding/etsi_its_mapem_ts_coding/include/etsi_its_mapem_ts_coding/mapem_ts_RegionalExtension.h:18,
                 from /home/james/zio/src/etsi_its_messages/etsi_its_coding/etsi_its_mapem_ts_coding/include/etsi_its_mapem_ts_coding/mapem_ts_AdvisorySpeed.h:63,
                 from /home/james/zio/src/etsi_its_messages/etsi_its_coding/etsi_its_mapem_ts_coding/src/mapem_ts_AdvisorySpeed.c:8:
/home/james/zio/src/etsi_its_messages/etsi_its_coding/etsi_its_mapem_ts_coding/include/etsi_its_mapem_ts_coding/mapem_ts_NodeOffsetPointXY.h:53:17: error: unknown type name ‘mapem_ts_Reg_NodeOffsetPointXY_
t’                                                                                                    
   53 |                 mapem_ts_Reg_NodeOffsetPointXY_t         regional;                            
      |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~                                              
gmake[2]: *** [CMakeFiles/etsi_its_mapem_ts_coding.dir/build.make:1378: CMakeFiles/etsi_its_mapem_ts_coding.dir/src/mapem_ts_AdvisorySpeed.c.o] Error 1

The file mapem_ts_NodeOffsetPointXY.h is included a second time by an import that occurs before the struct is defined, so it never gets to the definition and it fails.

I'm not sure how to break apart the circular import. Switching to the new release2 branch of the is_ts103301 repo didn't help - it just broke it elsewhere.

ASN1Type::BitString(_b) => todo!(),
ASN1Type::BitString(b) => (b.constraints.clone(), "uint8[]".into()),
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check if correct. Why did this never get raised for the other message types?

Same for change in utils/codegen/codegen-rust/rgen/src/msgs/utils.rs.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file is an example of a common problem in the generated message files. The ASN.1 definition is found in asn1/raw/is_ts103301/build/asn1/ISO-TS-19091-addgrp-C-2018-patched.asn.

ComputedLane ::= SEQUENCE {
  referenceLaneId    LaneID,
  offsetXaxis        CHOICE {
                        small   DrivenLineOffsetSm, 
                        large   DrivenLineOffsetLg
                        },  
  offsetYaxis        CHOICE {
                        small   DrivenLineOffsetSm, 
                        large   DrivenLineOffsetLg
                        },  
  rotateXY           Angle OPTIONAL, 
  scaleXaxis         Scale-B12 OPTIONAL, 
  scaleYaxis         Scale-B12 OPTIONAL, 
  regional  SEQUENCE (SIZE(1..4)) OF 
            RegionalExtension {{Reg-ComputedLane}} OPTIONAL,
  ... 
}

Note how the type ComputedLane contains inner types, e.g., for the member named offsetXaxis , which either is an DrivenLineOffsetSm named small or a DrivenLineOffsetLg named large. The types DrivenLineOffsetSm and DrivenLineOffsetLg are defined elsewhere, but an intermediate type wrapping this choice is needed.

Interestingly, these intermediate types are already generated and marked with a Inner type comment, however, they are put into the same ComputedLane.msg that this comment is referring to. The code generation function for this is defined here.

My current understanding is that we simply need to move these inner types into separate .msg files.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hopefully fixed with 8633eb7.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some files have weird -postfixes. Other examples are Reg.msg, LaneAttributes.msg, or NodeAttributeSet.msg.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hopefully fixed with 917c37c.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Naming of types of members of inner types is not yet correctly formatted, see

Would probably also prefer LaneAttributesRegional.msg instead of LaneAttributesregional.msg.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants