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

Revert "Command to allow geofence action to be set per-fence (#1482)" #1626

Merged
merged 1 commit into from
Apr 30, 2021

Conversation

bkueng
Copy link
Member

@bkueng bkueng commented Apr 30, 2021

This reverts commit 6652031.

There's a problem with the C library generator:
The addition of MAV_CMD to development.xml led to the removal
of MAV_CMD in common.h. This breaks build for everyone using that enum
and including 'common.h' or 'standard.h' (and probably others).
See mavlink/c_library_v2@992871b

@hamishwillee agreed to revert this until we have a correct fix.

This reverts commit 6652031.

There's a problem with the C library generator:
The addition of MAV_CMD to development.xml led to the removal
of MAV_CMD in common.h. This breaks build for everyone using that enum
and including 'common.h' or 'standard.h' (and probably others).
See mavlink/c_library_v2@992871b
@bkueng bkueng requested a review from julianoes April 30, 2021 09:12
@julianoes julianoes merged commit a278669 into master Apr 30, 2021
@julianoes julianoes deleted the revert_geofence branch April 30, 2021 09:14
@olliw42
Copy link
Contributor

olliw42 commented Apr 30, 2021

@bkueng
just in case it might be helpful
this is ultimately a problem of the generator, you can however often circumvent it by careful arrangement of the order of dialects here https://github.com/mavlink/mavlink/blob/master/scripts/update_c_library.sh#L63-L69
you need to choose the order such that "higher" dialects don't overwrite "lower" dialects
it can be logically understood but it is a bit weird, so a bit testing might be needed
I have not tested, but I guess I would try to swap the calls to standard and development

(few months ago I did a fix in mavgen but threw it away)(this issue was btw one reason I went away from mavgen LOL)

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

3 participants