-
Notifications
You must be signed in to change notification settings - Fork 13.6k
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
Adapted acceptance radius for rovers, and added vehicle_type field #11918
Conversation
db345cb
to
2e4a4d2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple of first comments but I need to go through it in detail again.
Overall this looks good, we just need to be careful with the usage surrounding VTOL where various modules depend on |
src/modules/commander/commander_tests/state_machine_helper_test.cpp
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks all correct now from what I can tell, thanks @ItsTimmy for all the fixes!
I have rebased this PR so it is ready to merge, if @dagar or @julianoes would like to review it again. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks correct to me. @dagar please review and/or merge.
@dagar any objections? |
Test data / coverage
I tested these changes in SITL, by running a mission from start to finish on the following targets:
gazebo_standard_vtol
gazebo_tiltrotor
gazebo_plane
gazebo_tailsitter
gazebo_iris
gazebo_rover
Describe problem solved by the proposed pull request
Many modules used
!vehicle_status.is_rotary_wing
to determine if a vehicle is fixed wing. This led to assuming that other vehicle types, like rovers, are fixed-wing.Describe your preferred solution
The
vehicle_status
message was changed to remove theis_rotary_wing
flag and replace it with an enum,vehicle_type
, which can take the valuesFIXED_WING
,ROTARY_WING
, orGROUND
Describe possible alternatives
I considered using the existing
system_type
field invehicle_status
, but this enum is too complex. There are many different types of rotary wing aircraft, for example. This field also does not consider the state of a VTOL; that is, whether it is in fixed-wing or rotary-wing mode.Additional context
This is a continuation from the work done in #7175