Airspeed selector follow-up #12887

sfuhrer · 2019-09-03T13:13:11Z

This PR enables the airspeed validation & selection module (#12353) by making the controllers listen to the airspeed_validated topic and no longer to the airspeed topic.

Changes:

VTOL_att_controller, FW_att_controller, FW_pos_controller subscribe to airspeed_validated topic
add ability to completely switch off airspeed failure detection (parameter ASPD_DO_CHECKS)
add ability to fallback to groundspeed-windspeed if airspeed sensor is declared invalid (param ASPD_FALLBACK)
remove airspeed checks from commander (since Airspeed validation & selection module #12353 was merged it was implemented also in this new module)
remove airspeed failsafe actions in commander (COM_ASPD_FS_ACT)
remove status.aspd_use_inhibit (the airspeed_validated topic airspeeds are set to NAN if not valid
rename all the params for the airspeed validation and move them from the COM to the ASPD section

After this gets merged, we should think about this stuff:

make it easy to switch between non-airspeed and normal flight (can we remove the CBRK_AIRSPD_CHECK? Rename/remove FW_ARSP_MODE?)
think about which airspeed should be fused into EKF (currently is still the "raw" airspeed from the first airspeed sensor)
improve wind estimator (tuning, resets) to get less false positives for the failure detection

Fixes: #13144.

sfuhrer · 2019-10-08T09:09:12Z

so far only sitl and bench tested, will go testflying soon

sfuhrer · 2019-10-08T16:07:55Z

Flight tested on standard VTOL: https://review.px4.io/plot_app?log=efbe527d-cb51-46e7-9264-ee0c0450494f

I set:
ASPD_PRIMARY = 1 (the airspeed sensor that's mounted)
ASPD_DO_CHECKS = 1 (enabling airspeed validity checks)
ASPD_FALLBACK = 1 (to use ground-windspeed as fallback if failure is detected)

At 3.30min I set the stallspeed to 100, thus triggering a failure detection. The selector then switched automatically to the groundspeed minus windspeed airspeed estimate (as defined in ASPD_FALLBACK = 1). No change change in behavior noticeable to the pilot, it kept flying in position control mode with airspeed and altitude controllable separately.
Note: very calm conditions, almost no wind.

msg/airspeed_validated.msg

src/lib/AirspeedValidator/AirspeedValidator.cpp

src/modules/airspeed_selector/airspeed_selector_main.cpp

RomanBapst · 2019-10-11T06:09:05Z

src/modules/vtol_att_control/standard.cpp

@@ -182,7 +182,7 @@ void Standard::update_vtol_state()
 		} else if (_vtol_schedule.flight_mode == vtol_mode::TRANSITION_TO_FW) {
 			// continue the transition to fw mode while monitoring airspeed for a final switch to fw mode
 			if (((_params->airspeed_disabled ||
-			      _airspeed->indicated_airspeed_m_s >= _params->transition_airspeed) &&


@sfuhrer I think we need to check for airspeed validity here as well. I assume we can just remove the check on "_params->airspeed_disabled" and check if airspeed is finite, correct?

Same holds for everywhere else in the vtol code.

I've added a PX4_ISFINITE(airpseed) everywhere airspeed is used, and if it's invalid it does the same as when "_params->airspeed_disabled". I've not yet removed the parameter, not 100% if we should do so with this PR, or if that's fuel for another PR (I was also thinking about removing the CBRK_AIRSPD_CHECK, but this then needs some further thoughts.

RomanBapst · 2019-10-17T05:27:27Z

@sfuhrer Please have a look at my last three commits. I cleaned up the airspeed usage for all three vtol types for the transition.
I had a lot of trouble reading what was there before (not your fault btw) and decided to change it.
I think previously it was also the case the even if the airspeed was NAN further down it was checked if airspeed was greater than the transition threshold.

RomanBapst · 2019-10-17T05:30:55Z

@sfuhrer I also rebased on master btw.

- FW attitude controller, FW position controller and VTOL attitude controller subscribe to airspeed_validated topic - add possibility to switch off the airspeed valid checks - remove airspeed valid checks from commander - clean up of VTOL transition logic Signed-off-by: Silvan Fuhrer <silvan@auterion.com>

…depending on number of connected sensors) but do it statically for the maximum number allowed. Check for number of connected sensors not only during start up, but always when vehicle is disarmed. Signed-off-by: Silvan Fuhrer <silvan@auterion.com>

dagar · 2019-11-14T14:47:41Z

src/modules/airspeed_selector/airspeed_selector_params.c

+ * @reboot_required true
+ * @group Airspeed Validator
+ */
+PARAM_DEFINE_INT32(ASPD_PRIMARY, 1);


We'll probably need to find a better way to do this later. We can't rely on the ordering being maintained in all cases.

Agree. As we currently though anyway just support 1 sensor it's not yet that critical.

dagar · 2019-11-14T14:48:44Z

src/modules/commander/Commander.cpp

@@ -578,7 +578,7 @@ Commander::Commander() :

 Commander::~Commander()
 {
-	delete[] _airspeed_fault_type;
+


You can delete this entirely, then in the header set the destructor to = default.

dagar · 2019-11-14T14:50:38Z

src/modules/fw_pos_control_l1/FixedwingPositionControl.cpp

-		    && PX4_ISFINITE(as.true_airspeed_m_s)
-		    && (as.indicated_airspeed_m_s > 0.0f)
-		    && !_vehicle_status.aspd_use_inhibit) {
+		if (PX4_ISFINITE(airspeed_validated.equivalent_airspeed_m_s)


I think we should be able to drop these checks and trust our "validated airspeed".

The way it works currently is that the airspeed validated is set to NAN inside the airspeed module if it is detected to be invalid (or user has disabled it by setting PRIMARY to -1). As I see it we either keep it this way and check for the finiteness of the airspeed_validated where it's used, or build in some logic in commander that sets a "airspeed_inhibited" flag in vehicle_status, or make this flag part of the airpseed_validated message (as it was at some point, but was removed because it then contains redundant information (setting it to NAN should be equal to "don't use airspeed)".
I'd vote for bringing it in as it is now.

That's fine as long as every consumer ends up with the same notion of airspeed validity.

dagar · 2019-11-14T15:32:43Z

I would change the work queue thread if this is now flight critical.

px4::wq_configurations::lp_default -> px4::wq_configurations::att_pos_ctrl

This is where the controllers and estimator run.

sfuhrer · 2019-11-14T16:22:03Z

I would change the work queue thread if this is now flight critical.

px4::wq_configurations::lp_default -> px4::wq_configurations::att_pos_ctrl

This is where the controllers and estimator run.

Sounds reasonable, changed it.

dagar · 2019-11-15T15:16:08Z

This needs to be included in px4_fmu-v2_default.

Note for later, we should probably do something to require the airspeed selector is active and healthy at arming for FW and VTOL (preflight check).

dagar · 2019-11-15T15:17:46Z

src/modules/commander/Commander.hpp

@@ -285,6 +270,7 @@ class Commander : public ModuleBase<Commander>, public ModuleParams
 	uORB::Subscription					_vehicle_acceleration_sub{ORB_ID(vehicle_acceleration)};

 	uORB::SubscriptionData<airspeed_s>			_airspeed_sub{ORB_ID(airspeed)};
+	uORB::SubscriptionData<airspeed_validated_s>			_airspeed_validated_sub{ORB_ID(airspeed_validated)};


Is any of this still being used in commander?

_airspeed_sub, _airspeed_validated_sub, _airspeed_fault_declared, _time_airspeed_fault_declared

…module is safety-critical Signed-off-by: Silvan Fuhrer <silvan@auterion.com>

…peed_validated subscription Signed-off-by: Silvan Fuhrer <silvan@auterion.com>

Signed-off-by: Silvan Fuhrer <silvan@auterion.com>

sfuhrer · 2019-11-15T17:03:24Z

This needs to be included in px4_fmu-v2_default.

Good point. v2 doesn't have enough flash (see CI failure). I had hoped that v2 would be sunset anyway soon (#13428), but as it seems this won't happen now. So how can we solve it, do you have open PRs that will free up flash @dagar ?

sfuhrer requested a review from RomanBapst September 3, 2019 13:13

sfuhrer self-assigned this Sep 3, 2019

sfuhrer added Fixed Wing 🛩️ Hybrid VTOL 🛩️🚁 Multirotor + Fixedwing! labels Sep 3, 2019

weekly-digest bot mentioned this pull request Sep 8, 2019

Weekly Digest (1 September, 2019 - 8 September, 2019) #12924

Closed

sfuhrer force-pushed the pr-AirspeedSelector_followup branch from f7c0748 to 9ba7418 Compare September 16, 2019 12:51

weekly-digest bot mentioned this pull request Sep 22, 2019

Weekly Digest (15 September, 2019 - 22 September, 2019) #13002

Closed

julianoes changed the title ~~[WIP] Pr airspeed selector followup~~ [WIP] Airspeed selector follow-up Sep 25, 2019

weekly-digest bot mentioned this pull request Sep 29, 2019

Weekly Digest (22 September, 2019 - 29 September, 2019) #13050

Closed

sfuhrer force-pushed the pr-AirspeedSelector_followup branch from 9ba7418 to a1cd0e5 Compare October 8, 2019 08:57

sfuhrer marked this pull request as ready for review October 8, 2019 09:09

sfuhrer changed the title ~~[WIP] Airspeed selector follow-up~~ Airspeed selector follow-up Oct 8, 2019

sfuhrer force-pushed the pr-AirspeedSelector_followup branch 2 times, most recently from 7a8cb3c to 08647c9 Compare October 8, 2019 14:29

sfuhrer mentioned this pull request Oct 8, 2019

Transition from airspeed mode to non airspeed mode #11619

Closed

AuterionWrikeBot assigned RomanBapst Oct 9, 2019

sfuhrer mentioned this pull request Oct 11, 2019

Aispeed sensor switching on SITL plane landings #13144

Closed

RomanBapst requested changes Oct 11, 2019

View reviewed changes

weekly-digest bot mentioned this pull request Oct 13, 2019

Weekly Digest (6 October, 2019 - 13 October, 2019) #13171

Closed

sfuhrer force-pushed the pr-AirspeedSelector_followup branch from deafc41 to cf02894 Compare October 13, 2019 20:44

dagar mentioned this pull request Oct 14, 2019

A couple of coverity fixes #13064

Merged

sfuhrer mentioned this pull request Oct 15, 2019

Airspeed_selector follow up issue #12503

Closed

sfuhrer force-pushed the pr-AirspeedSelector_followup branch from cf02894 to 280dcad Compare October 16, 2019 17:00

RomanBapst force-pushed the pr-AirspeedSelector_followup branch from 910e7dd to b1fb8a7 Compare October 17, 2019 05:30

sfuhrer force-pushed the pr-AirspeedSelector_followup branch 7 times, most recently from a8a61a6 to 63de4a0 Compare November 14, 2019 11:11

sfuhrer added 2 commits November 14, 2019 14:55

sfuhrer force-pushed the pr-AirspeedSelector_followup branch from 63de4a0 to 8e4992d Compare November 14, 2019 13:56

dagar reviewed Nov 14, 2019

View reviewed changes

sfuhrer force-pushed the pr-AirspeedSelector_followup branch from 4b0cdae to 9794449 Compare November 14, 2019 15:35

dagar reviewed Nov 15, 2019

View reviewed changes

sfuhrer added 3 commits November 15, 2019 17:33

Airspeed Selector: change work queue from lp to att_pos_ctrl as this …

49219e7

…module is safety-critical Signed-off-by: Silvan Fuhrer <silvan@auterion.com>

Commander: remove unused destructor, airspeed check function and airs…

0aef3b2

…peed_validated subscription Signed-off-by: Silvan Fuhrer <silvan@auterion.com>

add airspeed selector to px4_fmu-v2 defaults

f327c9b

Signed-off-by: Silvan Fuhrer <silvan@auterion.com>

sfuhrer force-pushed the pr-AirspeedSelector_followup branch from b8cd03d to f327c9b Compare November 15, 2019 16:38

Merge branch 'master' into pr-AirspeedSelector_followup

0ad7f13

dagar approved these changes Nov 21, 2019

View reviewed changes

dagar merged commit ebdc29b into master Nov 21, 2019

dagar deleted the pr-AirspeedSelector_followup branch November 21, 2019 19:17

mrpollo mentioned this pull request Mar 16, 2020

v1.11.0 Release Notes #14402

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Airspeed selector follow-up #12887

Airspeed selector follow-up #12887

sfuhrer commented Sep 3, 2019 •

edited by AuterionWrikeBot

Loading

sfuhrer commented Oct 8, 2019

sfuhrer commented Oct 8, 2019

RomanBapst Oct 11, 2019

RomanBapst Oct 11, 2019

sfuhrer Oct 13, 2019

RomanBapst commented Oct 17, 2019

RomanBapst commented Oct 17, 2019

dagar Nov 14, 2019

sfuhrer Nov 14, 2019

dagar Nov 14, 2019

sfuhrer Nov 15, 2019

dagar Nov 14, 2019

sfuhrer Nov 14, 2019

dagar Nov 14, 2019

dagar commented Nov 14, 2019

sfuhrer commented Nov 14, 2019

dagar commented Nov 15, 2019

dagar Nov 15, 2019

sfuhrer commented Nov 15, 2019

Airspeed selector follow-up #12887

Airspeed selector follow-up #12887

Conversation

sfuhrer commented Sep 3, 2019 • edited by AuterionWrikeBot Loading

sfuhrer commented Oct 8, 2019

sfuhrer commented Oct 8, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RomanBapst commented Oct 17, 2019

RomanBapst commented Oct 17, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dagar commented Nov 14, 2019

sfuhrer commented Nov 14, 2019

dagar commented Nov 15, 2019

Choose a reason for hiding this comment

sfuhrer commented Nov 15, 2019

sfuhrer commented Sep 3, 2019 •

edited by AuterionWrikeBot

Loading