Potential issues in accidentally merged commits

[sensei-hacker]

I accidentally merged commits from #11147 as part of #11230.
Which is unfortunate because there may be some issues with this code.

It's great that it uses the same function for the three different ways of setting the value. It's great to see the guards being used. There may be a couple issues, though:

A. calling internal nav task function

It mutates the state of the navigation task in a way that perhaps the set may be immediately overwritten.
Suppose this message is received and navigationSetAltitudeTargetWithDatum() runs at line +5227
On the next cycle, we have updateClimbRateToAltitudeController(0, 0, ROC_TO_ALT_CURRENT)
Or some stick XY stick movement (including releasing the stick to center) calls setDesiredPosition().
That can immediately overwrite the change, can't it?

In position mode, does the position mode altitude overwrite this?

We might need the nav task to consume an external variable (altitudeOverride) and handle it explicitly. See posControl.flags.forcedRTHActivated and LOGIC_CONDITION_OVERRIDE_THROTTLE

B. #ifdef USE_BARO vs `#ifdef USE_NAV

Maybe change #ifdef USE_BARO to #ifdef USE_NAV -- GPS is valid altitude source, and there may be others?

C. Undefined buffer read order

Perhaps more importantly, this line has undefined behavior:
navigationSetAltitudeTargetWithDatum((geoAltitudeDatumFlag_e)sbufReadU8(src), (int32_t)sbufReadU32(src));
I think there's an assumption there that the arguments will be evaluated in some order, but there's no telling which size chunk would be read first. To read U8 first, it needs to be a separate statement placed before the function call.

Originally posted by @sensei-hacker in #11147 (comment)