-
Notifications
You must be signed in to change notification settings - Fork 13.7k
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
weathervane: added configuration option via paramter #10883
Conversation
Just to clarify, we just need it disabled for takeoff as we see danger in having the vehicle transition in an arbitrary direction and it does not seem to have so much trouble during takeoff. We do require it for all other modes |
Please do not merge |
Signed-off-by: Roman <bapstroman@gmail.com>
- general enable / disable bit - auto takeoff enable / disable bit Signed-off-by: Roman <bapstroman@gmail.com>
@sanderux @dagar I updated the configuration possibilities. You can now enable weathervane generally for all position controlled modes, regardless of manual or mission. Additionally you can disable weathervane independently for auto takeoff. We can extend the bitmask in the future should we require more configuration options. |
return (bool)(_wv_config.get() & MASK_EN); | ||
} | ||
|
||
bool WeatherVane::should_run_in_mission(uint8_t position_setpoint_type) |
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.
What about only making this call on position setpoint updates?
@@ -44,8 +44,12 @@ | |||
* | |||
* @boolean | |||
* @group Multicopter Position Control | |||
* @min 0 | |||
* @max 3 | |||
* @bit 0 Enabled. |
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.
I'm wondering if this isn't exactly obvious that "Enabled" covers everything except for takeoff. Any input @hamishwillee?
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.
@RomanBapst I prefer "Enable" over "Enabled".
- I assume that if bit 0 is unset, it doesn't matter what the other bits are set to? i.e. to have on takeoff you must have both bit 0 and 1?
- You say this is active for all position control modes by default, except takeoff. I assume that means it is also active for landing and loiter by default?
Assuming that, how about:
/**
* Enable weathervane and configure use in optional flight modes.
*
* When enabled, weathervane is active for most position control modes.
* The additional bits can be used to enable weathervane for the remaining modes.
*
* @boolean
* @group Multicopter Position Control
* @min 0
* @max 3
* @bit 0 Enable.
* @bit 1 Enable for auto takeoff.
*/
PARAM_DEFINE_INT32(WV_CONFIG, 3);
We will also need to update https://docs.px4.io/en/config_vtol/vtol_weathervane.html
It seems to be getting a bit convoluted. To me it seems the simplest solution is: Either in bitmask or simple params I do not understand the added value of WV during manual modes. seems dangerous. |
What happened to setting it on the mission item? One way this could be kept simple is to have a single controller default weathervane setting (VTOL_TRANSITION_HEADING_VEHICLE_DEFAULT), but then set VTOL_TRANSITION_HEADING_TAKEOFF in your MAV_CMD_NAV_VTOL_TAKEOFF. https://mavlink.io/en/messages/common.html#MAV_CMD_NAV_VTOL_TAKEOFF https://mavlink.io/en/messages/common.html#VTOL_TRANSITION_HEADING |
@RomanBapst The suggestion from Daniel makes a lot of sense. |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
@RomanBapst Isn't this still valid? |
@hamishwillee yes, still valid |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
Closing as stale. |
@RomanBapst what's the status here? |
This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions. |
From my point okay to close this. We've addressed the main issue (arbitrary direction of transition) with #12630. It does now always align with direction of VTOL_TAKEOFF before transitioning. |
This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions. |
Fixed long time ago. |
@sanderux Requires the weathervane feature to be disabled for takeoff, landing and loiter.