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

Build ArduPilot against all.xml rather than ardupilotmega.xml #19929

Merged
merged 3 commits into from
Feb 8, 2022

Conversation

peterbarker
Copy link
Contributor

This has the advantage that we will understand and forward packets that we don't understand but are contained in all.xml.

This has the drawback that those checksums aren't free - there is a cost to being able to undersand (and thus route) mavlink messages.

@tridge tridge merged commit 408491d into ArduPilot:master Feb 8, 2022
@peterbarker peterbarker deleted the pr/build-against-all-xml branch February 8, 2022 00:11
@peterbarker
Copy link
Contributor Author

@olliw42 you might like to check the implications for storm32.xml

@hamishwillee for visibility

@hamishwillee
Copy link
Contributor

Thanks @peterbarker ! Doesn't this mean that ArduPilot is now carrying a lot of cruft? I kind of assumed you would build against all.xml for the GCS's but not the autopilot. Is this so you can get development.xml without having to think too much about it?

@olliw42
Copy link
Contributor

olliw42 commented Feb 8, 2022

@peterbarker

many thx for the ping, much appreciated !!

as much as I can see, storm32.xml is out-commented in the current ardupilot mavlink repo, so nfc.

In the upstream mavlink repo the external dialects are moved to the main xml folder, and included in all.xml. If ardupilot would do the same and build on all.xml, this would be a revolution! A very welcome revolution !!

storm32.xml includes ardupilotmega.xml, and I'm not sure how pymavlink handles "recursive" includes, but I assume this has been tested by some of you guys at some points, that pymavlink can handle such include trees.

If not, and you would decide to continue to exclude storm32.xml from all.xml, it would be not so much of an issue for me, since those who want to be serious with storm32 and mavlink appear to use BetaPilot anyhow.

so, please go along ! :):):)

thx for this

@peterbarker
Copy link
Contributor Author

Argh. I think we're going to have to revert this one for a while; ChibiOS is leaving a variable "arming_status" in the global namespace and that's causing us compilation issues as the ASLUAV dialect happens to contain a parameter with the name "arming_status". Srsly.

@peterbarker
Copy link
Contributor Author

as much as I can see, storm32.xml is out-commented in the current ardupilot mavlink repo, so nfc.

Not in our master branch, just in the mavlink commit referenced from our main repo.

Sadly, moving forward also happens to kill compilation against asluav... see other comment in here.

@peterbarker
Copy link
Contributor Author

peterbarker commented Feb 8, 2022

We're going to temporarily remove AVSUAS.xml in our copy of all.xml - hopefully returning it when we sort out the namespace conflict.

#20029
ArduPilot/mavlink#245

@peterbarker
Copy link
Contributor Author

peterbarker commented Feb 9, 2022 via email

@hamishwillee
Copy link
Contributor

Thanks. FWIW I think the inability to route messages that are not understood is something that should be looked at in the library itself. If not now, then "MAVLink 3".

@olliw42
Copy link
Contributor

olliw42 commented Feb 9, 2022

I think the inability to route messages that are not understood is something that should be looked at in the library itself. If not now, then "MAVLink 3".

mavlink 2.1 would be sufficient: mavlink/mavlink#1347
;)

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

Successfully merging this pull request may close these issues.

None yet

5 participants