mc_att_control: remove direct setting of att sp in Stabilized

[sfuhrer]

Currently in the MC attitude controller, in Stabilized mode, the generated attitude setpoints are copied inside the module to where they are used (plus also published over uORB).
Here I propose to remove this internal copying and only publish it to the uorb topic, which is subscribed to in the same module.

This "should" have no behavior change - unless we deem the passing of the attitude setpoint over uorb less stable and want to keep the internal copying of it through setAttitudeSetpoint(). I propose this change to be able to remove the "if(vtol_transition)" and keep the controller flows as consistent as possible.

Context: #22584 (comment)

FYI @MaEtUgR

[dagar]

uorb less stable and want to keep the internal copying of it through setAttitudeSetpoint(). I propose this change to be able to remove the "if(vtol_transition)" and keep the controller flows as consistent as possible.

It's not less stable or anything, but one problem with the change given the current structure is that now the attitude setpoint is delayed by one iteration. Probably not even noticeable, but it's unnecessary input latency we should be able to easily avoid with a slightly different structure.

Could you simply move "check for new attitude setpoint" to right before the attitude controller is run (_attitude_control.update(q))? You'll need to make sure the heading reset is still correct, but otherwise that seems fine.

PX4-Autopilot/src/modules/mc_att_control/mc_att_control_main.cpp

Lines 229 to 240 in 101d882

	// Check for new attitude setpoint
	if (_vehicle_attitude_setpoint_sub.updated()) {
	vehicle_attitude_setpoint_s vehicle_attitude_setpoint;
	if (_vehicle_attitude_setpoint_sub.copy(&vehicle_attitude_setpoint)
	&& (vehicle_attitude_setpoint.timestamp > _last_attitude_setpoint)) {
	_attitude_control.setAttitudeSetpoint(Quatf(vehicle_attitude_setpoint.q_d), vehicle_attitude_setpoint.yaw_sp_move_rate);
	_thrust_setpoint_body = Vector3f(vehicle_attitude_setpoint.thrust_body);
	_last_attitude_setpoint = vehicle_attitude_setpoint.timestamp;
	}
	}

[sfuhrer]

uorb less stable and want to keep the internal copying of it through setAttitudeSetpoint(). I propose this change to be able to remove the "if(vtol_transition)" and keep the controller flows as consistent as possible.

It's not less stable or anything, but one problem with the change given the current structure is that now the attitude setpoint is delayed by one iteration. Probably not even noticeable, but it's unnecessary input latency we should be able to easily avoid with a slightly different structure.

Could you simply move "check for new attitude setpoint" to right before the attitude controller is run (_attitude_control.update(q))? You'll need to make sure the heading reset is still correct, but otherwise that seems fine.

PX4-Autopilot/src/modules/mc_att_control/mc_att_control_main.cpp

Lines 229 to 240 in 101d882

	// Check for new attitude setpoint
	if (_vehicle_attitude_setpoint_sub.updated()) {
	vehicle_attitude_setpoint_s vehicle_attitude_setpoint;
	if (_vehicle_attitude_setpoint_sub.copy(&vehicle_attitude_setpoint)
	&& (vehicle_attitude_setpoint.timestamp > _last_attitude_setpoint)) {
	_attitude_control.setAttitudeSetpoint(Quatf(vehicle_attitude_setpoint.q_d), vehicle_attitude_setpoint.yaw_sp_move_rate);
	_thrust_setpoint_body = Vector3f(vehicle_attitude_setpoint.thrust_body);
	_last_attitude_setpoint = vehicle_attitude_setpoint.timestamp;
	}
	}

Good point. Done it in 6173323. I don't see an issue with it neither, neither with the order (run generate_attitude_setpoint() before the pulling of the attitude_setpoint), nor with that now the attitude_setpoint is only pulled when run_att_ctrl is actually true.

[mrpollo]

@MaEtUgR would appreciate your review with fresh eyes!
Note: Double check possibly yaw reset race conditions.