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

Move XML injection to the group level #15403

Merged
merged 2 commits into from
Jul 24, 2020
Merged

Move XML injection to the group level #15403

merged 2 commits into from
Jul 24, 2020

Conversation

hamishwillee
Copy link
Contributor

Param metadata is generated from source param definitions. The xmloutput parser also prefixes the generated parameter.xml file with additional "injected XML" that it pulls in from a local file. The problem with this is that this injected XML is not included in the generated markdown output, and would also need to be pulled into the json parameter output and any future output formats.

What this change does is import the injected XML into parameter groups and prepend these to the parameter group list before it is passed to the output generators. With this change the groups in the XML are added for any new output.

I have tested this and the output is the almost identical to the original XML.

  • Generated output is better in that output field order of injected content matches all other fields.
  • No fields are missing
  • The only thing missing is the new output is the group attribute no_code_generation - which is not in the "official" output - see <group name="UAVCAN Motor Parameters" no_code_generation="true">

The injected params are a subset of the original XML, so does not test all possible input. I therefore ran this to round trip the generated XML- it works for all that XML too.

Note however that it has the same limitations as the original parser - It does not check for duplication of groups/params in injected content. IMO not worth the effort.

@hamishwillee
Copy link
Contributor Author

So I guess that "no_code_generation="true">" thingy makes a difference :-(. Will look at it another day

Copy link
Member

@bkueng bkueng left a comment

Choose a reason for hiding this comment

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

Thanks, looks good.

Not sure how this will work in UAVCAN 1 though.

@hamishwillee
Copy link
Contributor Author

@bkueng Great. You'll have to merge too; I can't.

Not sure how this will work in UAVCAN 1 though.

Fair enough. But if this approach doesn't work for UAVCAN1 we'd probably have had to revisit anyway.

@bkueng bkueng merged commit 4ade248 into PX4:master Jul 24, 2020
@hamishwillee hamishwillee deleted the injectparam branch July 25, 2020 05:41
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.

None yet

2 participants