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

AST created with tree_from_a2l() does not contain layout for measurements. #22

Closed
LuDubies opened this issue Sep 27, 2023 · 7 comments
Closed

Comments

@LuDubies
Copy link

Missing LAYOUT property for measurements

When parsing A2L measurement objects, the ast does not contain layout information.

The property seems correctly defined in a2l-grpc/grammar/A2L.g4

Reproduce

When using the A2LParser.tree_from_a2l() on the attached dumb.txt (convert to .a2l), the properties of the "Dumbo" measurement contain no LAYOUT.
dumb.txt

This happens with a newly build pya2l and newest artifacts from a2l-grpc

@LuDubies
Copy link
Author

When comparing protobuf/A2L.proto to the ANTLR definition, the measurement type is missing the 4 properties:

  • discrete
  • layout
  • physUnit
  • symbolLink

The same is true for the AxisPts and Characteristic Types (physUnit & symbolLink)

@LuDubies
Copy link
Author

might close here and reopen in the a2l-grpc repo.

@Sauci
Copy link
Owner

Sauci commented Sep 30, 2023

Hi @LuDubies , thanks for the detailed feedback and the time, I'm on it :) Indeed those nodes are present in the grammar but not in the exchange protocol. The reason for that is because I'm used to have A2Ls with older ASAP2 version, where those nodes are not defined. I will fix it, and will close this issue once you will be able to parse your input as expected, is it ok for you?

Sauci pushed a commit that referenced this issue Oct 1, 2023
@Sauci
Copy link
Owner

Sauci commented Oct 1, 2023

@LuDubies it should be fixed in version 0.1.1. Could you confirm please? Thanks in advance for your feedback!

@LuDubies
Copy link
Author

LuDubies commented Oct 1, 2023

Hey @Sauci, thank you for the quick work.
I managed to extend the listener on friday but did not find the time to create a pull request.
Sorry that I had you do the same work again, but very grateful for the quick reaction.

I will check out your 0.1.1 release on wednesday and then we can close this.

@LuDubies
Copy link
Author

LuDubies commented Oct 1, 2023

Checking out the diffs for b9c4627, it looks like it should fix my issue and more. Can propably close this without me testing it again :)

Thanks again for your work, appreciate the project!

@LuDubies LuDubies closed this as completed Oct 1, 2023
@LuDubies
Copy link
Author

LuDubies commented Oct 4, 2023

Working with pya2l version 0.1.1 now, the issue has been fixed as predicted.
Just leaving this comment as confirmation.

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

No branches or pull requests

2 participants