ystero-dev / hampi

Rust ASN.1 Toolkit
Other
43 stars 18 forks source link

asn1 compiler failing to parse O-RAN KPM v3 specification #97

Closed gth828r closed 11 months ago

gth828r commented 11 months ago

I am running asn1-compiler 0.5.8 to generate code for the O-RAN KPM v3 specification, and it is failing with the following:

[2023-08-07T01:45:33Z INFO  asn1_compiler::compiler] Processing file: "kpm.asn1"
[2023-08-07T01:45:33Z WARN  asn1_compiler::parser::asn::types::constructed::choice] No tokens consumed in 1 iterations of the loop
[2023-08-07T01:45:33Z WARN  asn1_compiler::parser::asn::types::constructed::choice] No tokens consumed in 2 iterations of the loop
[2023-08-07T01:45:33Z ERROR asn1_compiler::parser::asn::defs] Failed to parse a definition at Token: Token { type: Identifier, span: Span { start: LineColumn { line: 35, column: 0 }, end: LineColumn { line: 35, column: 13 } }, text: "BinRangeValue" }
Error: Custom { kind: InvalidInput, error: "Parsing Error: Failed to parse a definition at Token: Token { type: Identifier, span: Span { start: LineColumn { line: 35, column: 0 }, end: LineColumn { line: 35, column: 13 } }, text: \"BinRangeValue\" }" }

I traced the problem down to new CHOICE types being defined with REAL variants. For example, this is the definition that initially causes a failure:

BinRangeValue ::= CHOICE {
    valueInt                INTEGER,
    valueReal           REAL,
    ...
}

Commenting out the valueReal REAL, line allows the parser to get past the issue for this definition. There are three such definitions in the file, and commenting out all three results in a successfully generated Rust file.

This is the command that I am running:

hampi-rs-asn1c --module kpm.rs --codec aper --derive debug --derive clone --derive eq-partial-eq -- kpm.asn1 e2sm_common_ies.asn1

Here are the two asn.1 files needed to try this: e2sm_common_ies.asn1.txt kpm.asn1.txt

Here is the original spec if it is of interest: O-RAN.WG3.E2SM-KPM-R003-v03.00.docx

gth828r commented 11 months ago

Upon further digging, it looks like the root issue is that the REAL primitive type is not supported yet.

gabhijit commented 11 months ago

@gth828r : Thanks for reporting this.

From what I remember yes the real type was not properly handled. I believe this is a good opportunity to do that. Will take a look. Do you think the CHOICE definition above can work as a simple test case for the same?

gth828r commented 11 months ago

Yes, I think you can drop it in for a unit test. Thanks for picking this up!

gabhijit commented 11 months ago

@gth828r : I have merged a PR that supports parsing and code generation for REAL types. This support is limited right now.

For example only value assignments of the type

pi REAL = 3.14 

are supported. Also, the codec support is stub only, so if there's an actual value that will be en/decoded it will panic with a todo!

You may want to try this and see if this works. If you confirm this works, I will release a new version for the crates, if indeed you need to encode/decode values, then will wait till the proper codec support is added. But if the Choice mentioned above is only sending INTEGER variants, this should still work.

If you've any trace with REAL values encoded, if you can add that - I can also add codec support and an example for that.

You may need to specify dependency as a git dependency.

gth828r commented 11 months ago

@gabhijit wow, that was quick! Thanks, I have verified that I can generate code using the latest master. However, the generated doesn't build, so I don't know if I'd cut a release. I'll leave that up to you.

I do not urgently need the ability to encode real values with this specification at this time, but I'm sure I will need it at some point in the future. Shall I open a separate issue for that which can be pursued at a slower time scale?

EDIT: noting that the generated code doesn't build.

gabhijit commented 11 months ago

@gth828r : Created #100 to track codec support. Also released a new crate version v0.5.9. If this looks fine, you might want to close this issue.

gth828r commented 11 months ago

That all sounds great to me, thanks again!