AC3.6-dev dual gps issues

I’ve played around with this configuration

config: AUAV-X2, one “normal” gps+mag on uart and i2c, one uc4h uavcan gps+mag on can, AC3.6-dev of tonight, latest MissonPlanner

GPS_AUTO_CONFIG = 0 or 1
GPS_AUTO_SWITCH = 0
GPS_TYPE = 1 or 2
GPS_TPYE2 = 9

and had these serious issues:

(i) all these configurations behaved the same

(ii) the Messages pannel in MP showed “GPS1: detected as UAVCAN at 19200 baud”, followed somewhat later by “GPS1: detected as ublox at 115200 baud”.

(iii) in no case a GPS2 has been detected

(iv) in the Status pannel in MP, the gps values for both gps1 and gps2 would always be zero, and gpshdop = 99.99 and gpshdop2 = 100

(v) very soon plenty of EKF switches do occur, which keeps on going

My feeling is that the gps1,gps2 detection scheme is totally broken.

Cheers, Olli

som screenshots:

i’ve reproduced this and have a pending PR to fix it here:

many thx, sir
I’ll test asap

I had a chance today to test this again

setup as before
AUAV-X2, one “normal” gps+mag on uart and i2c, one uc4h uavcan gps+mag on can, AC3.6-dev of tonight, latest MissonPlanner

this time I only tested for this two configs
GPS_AUTO_CONFIG = 0 or 1
GPS_AUTO_SWITCH = 0
GPS_TYPE = 2
GPS_TPYE2 = 9

I’m not convinced things are working as they should.

First run:
both ublox and uavcan gps connected => things look OK

Second run:
both ublox and uavcan gps connected, but then I disconnected the uavcan gps after startup, and reconnected it a longer while later => this does not look OK to me, eventhough gps2 was disconnected, gps1 gets reconfigured, and strangely enough as UAVCAN at 115200baud, once gps2 became reconnected, things settled correctly

Third run:
first only ublox was connected, after startup uavcan gps was connected => this does not look OK to me, initially gps1 is handled correctly, adding gps2 however also makes gps1 to “reenumerate” again

I could not notice a difference between GPS_AUTO_CONFIG = 0 or 1 (means, if there is any, I’ve not noticed).

In summary: Nice first try. But a too quick shoot. :slight_smile: To me it looks that first the logic should be worked out and/or specified, and then put into code.

Cheers, Olli

That’s how GPS enumeration works in AP. It has nothing to do with UAVCAN. Try different types of GPSs apart from UAVCAN and you will get the same results.
I am however concerned with test 2 and will try to figure out why that misnaming happens.

well, this might be in fact correct, and that what I describe isn’t related to having a uavcan gps
that was indeed an implied assumption of mine which might be a fallacy

but

this doesn’t make it any better.

I still would not call it proper behavior. I think that if gps2 is removed it should not lead to gps1 disappearing and running into an infinite loop of trys to enumerate gps1 correctly …

also: Being frightened to death (;)) I clearly have not attempted a flight, so I don’t know what would happen then. I would be very thankful if I could get a statement as regards what would happen during flight if one gps disappears … it also would make both gpses go and one gps to re-enumerate for indefinite? (can be answered with a plain yes or no)

cheers, Olli

@EShamaev The primary problem is copying the GPS state structure which has been null initilized in the UAVCAN backend is invalid. You need to at least update the state.instance variable. (Or better yet we ditch state.instance, and just make it a member variable of GPS_Backend).

@WickedShell Or better yet, the UAVCAN backend just copies the variables of the structure that it receives. Copying the entire structure isn’t a good behaviour.

@OXINARF: Agreed this is actually a large problem as the assumption in the GPS frontend is that it can use fields in GPS state freely, and that backends only fill in what they need. This can be reworked to not be the case, and we can make it a firm requirement for the future, but at the moment it is a real problem. I spent ~10 minutes perusing this when I noticed it after Olli’s report and it appears at the moment instance is the only one that is incorrect, but there could be something else lurking there.

Will get into that next week.

At least the Non-UAVCAN GPS enumeration is now preserved in master.