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

Enum parsing is a little broken #538

Open
hamishwillee opened this issue May 6, 2021 · 0 comments · May be fixed by #544
Open

Enum parsing is a little broken #538

hamishwillee opened this issue May 6, 2021 · 0 comments · May be fixed by #544

Comments

@hamishwillee
Copy link
Contributor

hamishwillee commented May 6, 2021

So the way this works is that when you do merge_enums() the x.enums for each xml file gets modified. If the file is the first to have an enum it gets to keep it and all the duplicate enum values are merged in. If the enum is "new" then it gets added to a list of new enums that get assigned back to the xml file. In other words, each XML file is stripped of any enums that are already merged into the top level.

What this means is that the top level file works fine - it has access to all enums it defined, including anything that is merged. However if you were to include mavlink.h from a dialect you would find it missing any enums that were declared in the parent, including any values that were in the XML.

This is different from other things like messages, where we use #defines to make sure that at each level you get what is underneath. I think this is a bug - i.e. that we make the assumption that when you generate a dialect you can use mavlink.h from any included dialect and use it as though you had build that included dialect directly.

Does this make sense - do we all agree it is a bug?

What it also means is that if you generate multiple dialect files the order matters - since you can remove the enums from a lower level

If so, any thoughts on fixing?

@hamishwillee hamishwillee linked a pull request May 20, 2021 that will close this issue
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 a pull request may close this issue.

1 participant