setNavigationFrequency not respected on custom packet branch

[nseidle]

I was using example 16 to check the max output rate on the new custom packet branch and noticed the setNav freq was not sticking. Reverting, it works again. There's something weird on this branch that prevents the changing of the nav frequency that I haven't pinned down yet.

[nseidle]

Code to replicate:

#include <Wire.h> //Needed for I2C to GPS

#include "SparkFun_Ublox_Arduino_Library.h" //http://librarymanager/All#SparkFun_Ublox_GPS
SFE_UBLOX_GPS myGPS;

void setup()
{
Serial.begin(500000); //Increase serial speed to maximize
while (!Serial)
; //Wait for user to open terminal
Serial.println("SparkFun Ublox Example");

Wire.begin();
Wire.setClock(400000);

if (myGPS.begin() == false) //Connect to the Ublox module using Wire port
{
Serial.println(F("Ublox GPS not detected at default I2C address. Please check wiring. Freezing."));
while (1)
;
}

myGPS.setI2COutput(COM_TYPE_UBX); //Set the I2C port to output UBX only (turn off NMEA noise)

byte rate = myGPS.getNavigationFrequency(); //Get the update rate of this module
Serial.print("Current update rate:");
Serial.println(rate);

Serial.println("Setting nav rate to 10");
myGPS.setNavigationFrequency(10); //Set output to 10 times a second

rate = myGPS.getNavigationFrequency(); //Get the update rate of this module
Serial.print("Current update rate:");
Serial.println(rate);
}

void loop()
{

}

[nseidle]

So it's not related to the recent saveConfiguration changes.

[PaulZC]

I saw some weirdness late last night that showed that the set-dynamic-model-with-a-custom-packet feature wasn't sticking either. I had debug turned on and the packet contents all looked OK but when I re-ran the code (without doing a reset or removing the power) the dynamic model jumped back to the old value. It seems to be platform and/or interface dependent. I shall investigate further...
Also we're not completely done with this branch yet. I slept on it and have realized that I'm not currently checking if the 'data' packet (packetCfg or customCfg) has been or is being overwritten (e.g. by a PVT). The new classAndIDmatch plus flow-down of requested class and ID means that we can do that. I will add a new return value to the enum saying something like OVERWRITTEN.

[PaulZC]

Oh, this is a good one...
The dynamic model does not stick if I use the new custom packet to change it, but only when I'm using an ATmega328P to talk to the ZED over I2C.
If I use an Artemis Thing Plus running exactly the same code, it sticks fine.
And, just to make things really confusing, on the ATmega328P, it does appear to stick while the code is running but then un-sticks itself if I close the serial monitor and reopen it.
I shall keep digging...

[PaulZC]

My weirdness appears to be voltage related. I'm using an ATmega328P RedBoard Qwiic hooked up to a ZED through the Qwiic port. I had the IO voltage set to 5V. But that's OK as the Qwiic pins reduce the voltage to 3.3V. Closing and reopening the serial monitor was - I think - causing the ZED to reset. When I changed the IO voltage to 3.3V, the problem went away.

Your issue is due to setNavigationFrequency being - I think - the only function that does a read-modify-write without updating the packetCfg.len before doing the write. Now that should not matter. Doing the read should store the correct length in packetCfg.len. But for reasons I don't yet understand .len was still zero. The write then became another read. The (temporary) fix is to add:

packetCfg.len = 6; // Bug fix!

just before the

return ((sendCommand(packetCfg, maxWait)) == SFE_UBLOX_STATUS_DATA_SENT);

I want to understand the root cause of this and I don't at the moment.

[PaulZC]

OK. Now I understand...
Previously, we were using packetCfg as a true global. We were passing the global around as if it were a pointer:

sendCommand(packetCfg);

WaitForNoACKResponse also made use of packetCfg as a global, and wasn't passed a copy or a pointer.

After my updates, WaitForNoACKResponse now receives a pointer to the packet to allow it to make use of a custom packet instead of packetCfg. Doing it this way means that the global nature of packetCfg broke down and you could not rely on it containing valid data after sendCommand. Hence the .len being zero when it should be six in setNavigationFrequency.

The solution is to use pointers everywhere... And involves changing sendCommand so it expects a pointer to a packet, not a packet.

It seems to work OK - but it is a big change. And will cause backward compatibility problems for anyone that was calling sendCommand from their own .ino.

I think the new way of doing things is better. But we should discuss.

Please try the latest commit if you've got time. setNavigationFrequency now works as expected.

[PaulZC]

Closing... This was due to a combination of: packetCfg not being a true global any more (in the two most recent branches); and setNavigationFrequency being - I think - the only function that didn't update the packet length before doing the second sendCommand.