core: frontend: onboardSensors: fix internal baro showing as water pressure sensor

[Williangalvani]

tested

[ES-Alexander]

Nice!

Per my comment in #2567, this fixes a bug but is only a partial fix for the observed behaviour, because we’re saying MS5611 is a water sensor (to handle the current misrepresentation of the MS5837 on the Bar30), and the Pixhawk has a genuine MS5611 as its onboard barometer (so that one will still show up as water unless we handle it explicitly). Alternatively we could stop treating MS5611 as water except in the special case where we expect it’s actually an MS5837, e.g. when it’s used on a sub vehicle and connected via I2C instead of SPI.

[Williangalvani]

Nice!

Per my comment in #2567, this fixes a bug but is only a partial fix for the observed behaviour, because we’re saying MS5611 is a water sensor (to handle the current misrepresentation of the MS5837 on the Bar30), and the Pixhawk has a genuine MS5611 as its onboard barometer (so that one will still show up as water unless we handle it explicitly). Alternatively we could stop treating MS5611 as water except in the special case where we expect it’s actually an MS5837, e.g. when it’s used on a sub vehicle and connected via I2C instead of SPI.

hmmm makes sense! we can check if it is on i2c and connected to bus BARO_EXT_BUS

[patrickelectric]

@ES-Alexander what is the current status of this PR ? Should we merge it ?

[ES-Alexander]

what is the current status of this PR ? Should we merge it ?

@patrickelectric it wouldn't hurt to merge it as-is, but I think it makes more sense for the is_water_baro function to get updated as part of this PR, with the additional check that's described in the discussion here, since otherwise this PR isn't particularly useful (and I believe it should be a pretty small change).

I don't know how to actually do those checks, but I think an appropriate pseudocode update would be something like:

        if (['MS5837', 'MS5611', 'KELLERLD'].includes(compass.deviceName ?? '--')
        && "baro is on I2C, on BARO_EXT_BUS"
        && autopilot.vehicle_type === 'Submarine') {
          results[compass.param] = true
        }

Even better if it actually reports the sensor as water type, instead of us guessing from how it's connected.

Also @Williangalvani

1. I'm not sure why that's currently checking compasses instead of baros - is that check actually against all connected sensors (and the variable is just poorly named), or is that another bug?
2. Would it make sense to also check for boats, or do they not yet have support for water sensors?

[ES-Alexander]

Confirmed working as expected! :-D

Nice work - this one ended up being more of a slog than expected...

[ES-Alexander]

Vehicle setup overview docs may need an updated screenshot.