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 values larger than Java max int #678

Closed
wants to merge 2 commits into from
Closed

Conversation

radarku
Copy link

@radarku radarku commented May 20, 2022

Fix to the Java generator for enum values that are larger than Java maximum integer size.

Without this PR, mavgen would produce MAVLink library code that cannot compile. This change preserves existing interoperability (i.e., current source code that expects integer enums and int types for Java switch statements), while also allowing larger enum values.

@radarku
Copy link
Author

radarku commented May 23, 2022

@peterbarker and @magicrub - @TSC21 mentioned that this might be a slight improvement (compatibility-wise) to https://github.com/ArduPilot/pymavlink/pull/639/files

@magicrub
Copy link
Contributor

no objection here. If it compiles and works then ok

@radarku
Copy link
Author

radarku commented May 25, 2022

Hi @magicrub - I tested against the current RAS-A IOP Specification and it generates Java message definitions that compile.

@magicrub
Copy link
Contributor

@radarku woah, didn't realize this is still pending.. sorry about that! Thanks for updating it with master with a merge but we don't merges in PRs. Please rebase on master instead.

@tridge
Copy link
Contributor

tridge commented Apr 25, 2023

@radarku needs rebase due to conflicts

@shancock884
Copy link
Contributor

Note that this PR appears to be duplicated by #744. I believe this (or 744) would fix #825.

@peterbarker
Copy link
Contributor

@radarku I've merged another PR which was essentially identical to this one - simply because this one needs rebasing and the other didn't.

Thanks for looking at this!

(and thanks for @shancock884 for sorting all this out again)

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

Successfully merging this pull request may close these issues.

None yet

7 participants