Git Product home page Git Product logo

Comments (86)

RobMeades avatar RobMeades commented on September 23, 2024 3

I have raised zephyrproject-rtos/zephyr#69290 as this seems to pretty clearly be an issue with the Zephyr STM32 I2C driver. A quick fix would be to comment out this line:

https://github.com/zephyrproject-rtos/zephyr/blob/53f527cc2dc322302a11e2a524126e62a4dc6834/drivers/i2c/i2c_ll_stm32.c#L160

...in the Zephyr STM32 I2C driver.

from ubxlib.

RobMeades avatar RobMeades commented on September 23, 2024 2

Find here:

https://github.com/u-blox/ubxlib/tree/preview_zephyr_i2c_port_rmea

...a preview branch of the change which implements the I2C transfer in a new way for Zephyr, a way which should work for STM32. I've tested it manually and it works on my STM32F7 Nucleo F767ZI board. I have also checked that it does not break nRF52/nRF53. Our test system is not yet set up with Zephyr on an STM32 board which has a GNSS device attached via I2C, so I can't do comprehensive testing just yet.

@djfurie, @AarC10 / @Naquino14: when you have a moment, please check if this works in your Zephyr/STM32/I2C configurations.

Once you have both confirmed that it is good I will get the change reviewed, merged and pushed to master here; I will update this issue when that is done and delete the preview branch a little while after that.

from ubxlib.

Naquino14 avatar Naquino14 commented on September 23, 2024 1

Looking at your device tree stuff, I notice you have:

   max0: maxm10s@42 {
        status = "okay";
        compatible = "u-blox,maxm10s";
        reg = <0x42>;
    };

...in the I2C entry, which I guess is for another driver inside Zephyr. Is it possible that driver is somehow still trying to do something, might somehow be active? Not that it should cause i2c_transfer() to return an error, of course, just thought I'd point it out.

Aaron and I were working on this together. When I removed it (at the time I was also only calling uDeviceOpen in init_maxm10s per the requirements of /port/platform/zephyr/Readme.md) uDeviceOpen returned -2. It would be helpful to test again though, so I will pull Aaron's changes and delete that node in the tree and let you know what I find.

from ubxlib.

AarC10 avatar AarC10 commented on September 23, 2024 1

To follow up, we added some inits so the mutexes would get initialized so it stopped returning -2, so this no longer seems to be an issue. We also are not using any other outside driver, so I guess that the GPS definition inside i2c1 is unnecessary. We can use a Saleae to try probing UART to double check, but probing I2C will not be possible on this board. We could also try to get a breakout and write an overlay for a F446 nucleo and probe those lines and report back then if nothing else seems to be of concern.

from ubxlib.

djfurie avatar djfurie commented on September 23, 2024 1

Can you try adding i2c-address = <0x42>; to your device tree definition for the max0?

    gps {
        status = "okay";
        compatible = "u-blox,ubxlib-device-gnss";
        transport-type = "i2c2";
        i2c-already-open;
        i2c-address = <0x42>;
        module-type = "U_GNSS_MODULE_TYPE_M10";
    };

I believe that there may be a bug in the DT bindings that default the i2c address to 42 instead of 0x42

Edit: fixing the address only got me a few lines further in the code (to line 2372). However, without specifying 0x42 I was seeing NACKs, now I'm seeing valid traffic on I2C. But it times out while polling for whatever it's looking for.

Edit2:
Am I interpreting the packet right? Class = 0x0A ID = 0x06? I'm not seeing this message in the documentation.
image

gdb) where
#0  sendReceiveUbxMessage (pInstance=0x20008e48, messageClass=<optimized out>, messageId=<optimized out>, pMessageBody=<optimized out>, messageBodyLengthBytes=messageBodyLengthBytes@entry=0, pResponse=pResponse@entry=0x200052e0 <_k_thread_stack_mainThread_id+3424>)
    at /home/dan/projects/gps_project/firmware/deps/ubxlib/gnss/src/u_gnss_private.c:700
#1  0x0801b870 in uGnssPrivateSendReceiveUbxMessage (pInstance=pInstance@entry=0x20008e48, messageClass=messageClass@entry=10, messageId=messageId@entry=6, pMessageBody=pMessageBody@entry=0x0, messageBodyLengthBytes=messageBodyLengthBytes@entry=0, pResponseBody=<optimized out>, 
    pResponseBody@entry=0x20005310 <_k_thread_stack_mainThread_id+3472> "", maxResponseBodyLengthBytes=maxResponseBodyLengthBytes@entry=120) at /home/dan/projects/gps_project/firmware/deps/ubxlib/gnss/src/u_gnss_private.c:2555
#2  0x0801b9ea in uGnssPrivateSendOnlyCheckStreamUbxMessage (pInstance=pInstance@entry=0x20008e48, messageClass=messageClass@entry=6, messageId=messageId@entry=4, pMessageBody=pMessageBody@entry=0x200053c0 <_k_thread_stack_mainThread_id+3648> "", 
    messageBodyLengthBytes=messageBodyLengthBytes@entry=4) at /home/dan/projects/gps_project/firmware/deps/ubxlib/gnss/src/u_gnss_private.c:2358
#3  0x0800492a in uGnssPwrOn (gnssHandle=0x20008e08) at /home/dan/projects/gps_project/firmware/deps/ubxlib/gnss/src/u_gnss_pwr.c:578
#4  0x08004b16 in addDevice (gnssTransportHandle=gnssTransportHandle@entry=..., deviceTransportType=<optimized out>, pCfgGnss=pCfgGnss@entry=0x200054f0 <_k_thread_stack_mainThread_id+3952>, pDeviceHandle=pDeviceHandle@entry=0x200054e4 <_k_thread_stack_mainThread_id+3940>)
    at /home/dan/projects/gps_project/firmware/deps/ubxlib/common/device/src/u_device_private_gnss.c:181
#5  0x08004c3a in uDevicePrivateGnssAdd (pDevCfg=pDevCfg@entry=0x200054e8 <_k_thread_stack_mainThread_id+3944>, pDeviceHandle=pDeviceHandle@entry=0x200054e4 <_k_thread_stack_mainThread_id+3940>) at /home/dan/projects/gps_project/firmware/deps/ubxlib/common/device/src/u_device_private_gnss.c:303
#6  0x08002a6e in uDeviceOpen (pDeviceCfg=0x0, pDeviceHandle=pDeviceHandle@entry=0x20001ca0 <gps_device_handle>) at /home/dan/projects/gps_project/firmware/deps/ubxlib/common/device/src/u_device.c:289
#7  0x080070f2 in gps_init () at /home/dan/projects/gps_project/firmware/myprojectname/application/src/gps.c:39
#8  0x08006086 in mainThread () at /home/dan/projects/gps_project/firmware/myprojectname/application/src/main.c:58
#9  0x080078ce in z_thread_entry (entry=0x200008d4 <log_dynamic_app>, p1=0x0, p2=0x0, p3=0x0) at /home/dan/projects/gps_project/firmware/deps/zephyr/lib/os/thread_entry.c:48
#10 0xaaaaaaaa in ?? ()

So I'm very confident I'm looking at the same issue, and have confirmed that what is actually going out over the wire is wrong. I believe the intended message is a UBX-CFG-RST (0x06 0x04).

from ubxlib.

djfurie avatar djfurie commented on September 23, 2024 1

Thanks for the message sequence. That will be helpful for debugging further, as I'm obviously a bit confused about what messages should be passing when =)

from ubxlib.

cturvey avatar cturvey commented on September 23, 2024 1

UBX-MON-MSGPP is supported on SPG 5.00 and SPG 5.10

0x20910196 CFG-MSGOUT-UBX_MON_MSGPP_I2C
0x20910197 CFG-MSGOUT-UBX_MON_MSGPP_UART1
0x2091019A CFG-MSGOUT-UBX_MON_MSGPP_SPI

from ubxlib.

djfurie avatar djfurie commented on September 23, 2024 1

In my situation, I'm asserting the hardware reset line just prior to initialization, so I wouldn't expect a full buffer:

int gps_init() {
    int rc;

    // Toggle the GPS reset pin
    gpio_pin_configure_dt(&gps_enable, GPIO_OUTPUT_ACTIVE);
    k_sleep(K_MSEC(10));
    gpio_pin_set_dt(&gps_enable, 0);
    k_sleep(K_MSEC(500));

    rc = uPortInit();
    if (rc != 0) {
        return rc;
    }

    rc = uPortI2cInit();
    if (rc != 0) {
        return rc;
    }

    rc = uDeviceInit();
    if (rc != 0) {
        return rc;
    }

    rc = uDeviceOpen(NULL, &gps_device_handle);
    if (rc != 0) {
        return rc;
    }

    rc = uGnssPosGetStreamedStart(gps_device_handle, U_GNSS_POS_STREAMED_PERIOD_DEFAULT_MS, callback);

    return rc;
}

Based on the info here, I'm going to look at this as a potential hardware issue (maybe a conflicting i2c address? I have no clue) for now. I'll be curious to know if the i2c-address fix for the DT binding fixes @AarC10's issue. I'll chime back in if I discover anything interesting.

from ubxlib.

Naquino14 avatar Naquino14 commented on September 23, 2024 1

Hey! I haven't added the resetting code yet, and instead commented out the max0 node and edited the cfg-device-gnss node to look like this:


    cfg-device-gnss {
        compatible = "u-blox,ubxlib-device-gnss";
        status = "okay";
        transport-type = "i2c1";
        module-type = "U_GNSS_MODULE_TYPE_M10";
        i2c-already-open;         // added this
        i2c-address = <0x42>; // and this
    };

Im currently stepping through gdb to see what goes wrong, as its either hanging or zephyr is exiting the init thread.

...if it can't find your i2c1 binding, and the only reason I can see that it would do that is if CONFIG_I2C is not defined for the build

CONFIG_I2C is set to y here.

from ubxlib.

Naquino14 avatar Naquino14 commented on September 23, 2024 1

@RobMeades
it sends b5 62 0a 06 00 00 10 3a and then says uDeviceOpen returned -8.

from ubxlib.

AarC10 avatar AarC10 commented on September 23, 2024 1

Screenshot_20240210_121050

from ubxlib.

RobMeades avatar RobMeades commented on September 23, 2024 1

Anything is possible, just not likely since, from the debug prints, the GNSS device's I2C interface is working (i.e. it has acknowledged b5 62 0a 06 00 00 10 3a).

from ubxlib.

Naquino14 avatar Naquino14 commented on September 23, 2024 1

I wish it was possible at this time to probe the I2C lines... Our hardware team didn't include test points for it.
I will carve out some time Today and or Tuesday to debug and find the contents of that buffer on line, 2008. I also still haven't tried to reset the chip beforehand so I will try my best to do that as well.
By the way, thank you very much for your patience!

from ubxlib.

cturvey avatar cturvey commented on September 23, 2024 1

It is possible to remap the I2C SCL/SDA to the UART1 RX/TX, either temporarily, or permanently. This can be helpful with some of the off-the-shelf UAV modules which only export the UART1, and a lot of systems want to use I2C due to lack of UARTs on their MCU, or simply preferring the multi-drop, simple cabling of I2C

I2C on UART pins PIO (TX=SCL, RX=SDA)
 0x10510003 CFG-I2C-ENABLED = 1
 0x10510004 CFG-I2C-REMAP = 1
 0x10520005 CFG-UART1-ENABLED = 0
 
Or with UART Enabled but UART/I2C pins swapped (SWAP TX/SCL RX/SDA)
 0x10520005 CFG-UART1-ENABLED = 1  
 0x10520004 CFG-UART1-REMAP = 1
 0x10510003 CFG-I2C-ENABLED = 1
 0x10510004 CFG-I2C-REMAP = 1

https://portal.u-blox.com/s/question/0D52p0000E8WgehCQC/how-to-talk-to-maxm10s-over-i2c
https://portal.u-blox.com/s/question/0D52p0000DcUjwwCQC/how-to-check-if-ubxm10050kb-device-contains-flash

from ubxlib.

RobMeades avatar RobMeades commented on September 23, 2024 1

...it says that the I2C clock is 100 kHz in the debug print out above. Remember that you get the default values applied by ubxlib, not the values from the &i2cx entry, so to get 400khz you would need to have an i2c-clock-hertz entry with value 400000.

from ubxlib.

Naquino14 avatar Naquino14 commented on September 23, 2024 1

Yikes, let me change it really quick. If that doesn't work ill attempt this expiriment.

from ubxlib.

Naquino14 avatar Naquino14 commented on September 23, 2024 1

Im testing it more right now. Please hold...

from ubxlib.

Naquino14 avatar Naquino14 commented on September 23, 2024 1

I guess I got lucky that time... Getting error -8 from uDeviceOpen() again

from ubxlib.

RobMeades avatar RobMeades commented on September 23, 2024 1

I do have a similar ST MCU board and a sparkfun breakout of the MAX-M10s chip. I could talk to Aaron about setting it up.

Probably print out the values first, then try that setup: should be pretty trivial to create, in case this is somehow a SW issue, but it smells of an I2C HW-level issue to me, getting access to the I2C lines so that we can monitor them directly is likely the thing.

from ubxlib.

bch2857 avatar bch2857 commented on September 23, 2024 1

Hi, hardware man here. Unfortunately I am out of town until tomorrow night so I won't be able to get those leads until then or Monday morning

from ubxlib.

Naquino14 avatar Naquino14 commented on September 23, 2024 1

I'm not using a genuine saleae (I got it off Amazon for $12) so I'm not sure. Give me a few minutes to zip up that file.

from ubxlib.

RobMeades avatar RobMeades commented on September 23, 2024 1

@djfurie: what MCU are you using? If the "P" in an orange circle in your logic analyzer's trace means "stop bit" you may have the same issue.

from ubxlib.

AarC10 avatar AarC10 commented on September 23, 2024 1

Sounds good to me. Thank you. We will be happy to help you test these changes. Let us know if there's anything else you need that could help.

from ubxlib.

AarC10 avatar AarC10 commented on September 23, 2024 1

Looks good! Nice. Thank you for your help and patience. We greatly appreciate it
Screenshot_20240226_225617

from ubxlib.

RobMeades avatar RobMeades commented on September 23, 2024 1

Great, thanks for checking. Feel free to use the Zephyr APIs instead if that is easier for you, we don't care really, we just sell modules :-).

from ubxlib.

RobMeades avatar RobMeades commented on September 23, 2024 1

Since it worked for AarC10 and Naquino14, I'm going to take a chance that it will also work for you @djfurie (please let me know if not): the commit containing the fix is now on master here, see 6d12d9f. I will delete the preview branch sometime next week.

from ubxlib.

Naquino14 avatar Naquino14 commented on September 23, 2024 1

Thank you again for all the help!

from ubxlib.

RobMeades avatar RobMeades commented on September 23, 2024

Hi there, and sorry you're having trouble with this.

Your code and your configuration looks good to me and, from the evidence of the U_PORT_BOARD_CFG prints, the configuration seems to be being adopted by this code. The failure cases between UART and I2C are different in that, in the UART case, this code believes it has sent b5 62 0a 06 00 00 10 3a and has timed-out after waiting 10 seconds for the response, while for the I2C case the "sent command:" debug print is missing and the error is pretty much immediate.

That said, it looks as though your gdb screenshot is from the I2C case, which says that, even in that case, it is still getting all the way to trying to send the command, which implies that this line is likely returning U_ERROR_COMMON_DEVICE_ERROR, which can only happen if i2c_transfer() is returning an error.

As a next data point, at least for the I2C case, it is probably worth trying to find out what i2c_transfer() is returning.

Do you happen to be able to sniff the I2C or UART lines at all? Not necessary yet, but might turn out so. EDIT: I see you have already said that you can do this above. It would be interesting to know, in either the UART or I2C cases, if you can see the MCU sending the command to the GNSS device.

from ubxlib.

RobMeades avatar RobMeades commented on September 23, 2024

Looking at your device tree stuff, I notice you have:

   max0: maxm10s@42 {
        status = "okay";
        compatible = "u-blox,maxm10s";
        reg = <0x42>;
    };

...in the I2C entry, which I guess is for another driver inside Zephyr. Is it possible that driver is somehow still trying to do something, might somehow be active? Not that it should cause i2c_transfer() to return an error, of course, just thought I'd point it out.

from ubxlib.

RobMeades avatar RobMeades commented on September 23, 2024

Do you happen to be able to sniff the I2C or UART lines at all? Not necessary yet, but might turn out so.

I see you have already said that you can do this above. It would be interesting to know, in either the UART or I2C cases, if you can actually see the MCU sending the command b5 62 0a 06 00 00 10 3a to the GNSS device.

from ubxlib.

RobMeades avatar RobMeades commented on September 23, 2024

Thanks: there really shouldn't be a need to probe HW lines for something this basic: there's just something misaligned somewhere between what ubxlib thinks is connected, what Zephyr thinks is connected, and what is actually connected.

For I2C, getting the return value of i2c_transfer() should tell us quite a lot I hope.

from ubxlib.

djfurie avatar djfurie commented on September 23, 2024

I see in the code that it should be a UBX-MON-MSGPP that is being sent.

That type doesn't appear in the M10 software guide. Appears that it's not supported?
https://content.u-blox.com/sites/default/files/u-blox-M10-SPG-5.10_InterfaceDescription_UBX-21035062.pdf

It's a defined message for the M8 series:
https://content.u-blox.com/sites/default/files/products/documents/u-blox8-M8_ReceiverDescrProtSpec_UBX-13003221.pdf

from ubxlib.

RobMeades avatar RobMeades commented on September 23, 2024

I believe that there may be a bug in the DT bindings that default the i2c address to 42 instead of 0x42

Woohoo! Well spotted: when I end-to-end tested this I did so on SPI (thinking that was the most complex case) not on I2C so that would have got past me.

On the opening message sequence, this is what it should look like; this is a log from talking to an M10, so it was certainly there in this FW version (FWVER=SPG 5.10, PROTVER=34.10):

U_GNSS: sent command b5 62 0a 06 00 00 10 3a.
U_GNSS: decoded UBX response 0x0a 0x06: 0a 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 [body 120 byte(s)].
U_GNSS: sent command b5 62 06 04 04 00 00 00 09 00 17 76.
U_GNSS: sent command b5 62 0a 06 00 00 10 3a.
U_GNSS: decoded UBX response 0x0a 0x06: 0c 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 [body 120 byte(s)].

I will investigate...

from ubxlib.

djfurie avatar djfurie commented on September 23, 2024

Will revisit this tomorrow, but my last data point is that I don't seem to get any response after the UBX-MON-MSGPP is sent. I just observe reads of the 0xFD register, which returns 0xFF 0xFF, and then a lot of bytes read from the 0xFF register. This repeats for a while until things time out.

image

from ubxlib.

RobMeades avatar RobMeades commented on September 23, 2024

You're absolutely right that the MAX-M10 interface manual doesn't mention the UBX-MON-MSGPP message, which I hadn't noticed 'cos the MAX-M10 devices we have in the ubxlib test system, devices which report FWVER=SPG 5.10, PROTVER=34.10 (which are the versions the interface manual says it is for), respond to it.

EDIT: curiously the interface manual does include the configuration message to configure the rate of the UBX-MON-MSGPP message on I2C, SPI and UART (CFG-MSGOUT-UBX_MON_MSGPP_XXX).

Very odd. I will enquire with people who should know tomorrow.

from ubxlib.

RobMeades avatar RobMeades commented on September 23, 2024

One more thing: in your I2C trace, a read from register 0xFD produces the response 0xFF 0xFF.

If there were no data to be read, I would expect it to produce the response 0x00 0x00; see below a portion from an I2C trace I happened to take a few days ago with a MAX-M10S:

image

The response you are getting would appear to mean that there are at least 65535 bytes of data to be read, which might take a little while, possibly explaining the timeout?

I guess a fix for this would be to not do the UBX-MON-MSGPP either side of the UBX-CFG-RST, i.e. to call uGnssPrivateSendOnlyStreamUbxMessage() instead of uGnssPrivateSendOnlyCheckStreamUbxMessage() just here:

// The message is not acknowledged, so must use
// uGnssPrivateSendOnlyCheckStreamUbxMessage()
message[2] = 0x09; // Controlled GNSS hot start
if (uGnssPrivateSendOnlyCheckStreamUbxMessage(pInstance,

That would make it more of a "best effort" reset but it would then work in situations where the GNSS chip might have been left on and accumulating messages for some time before we are attached to it.

from ubxlib.

RobMeades avatar RobMeades commented on September 23, 2024

A fix to apply the correct default I2C address is available in a preview branch here:

https://github.com/u-blox/ubxlib/tree/preview_fix_zephyr_dts_gnss_rmea

...should anyone need it. I will update this issue when the fix is pushed to the master branch and will delete the preview branch some time after that.

EDIT: fix now pushed to master here in commit fdc78b8.

from ubxlib.

RobMeades avatar RobMeades commented on September 23, 2024

Thanks for confirming @cturvey, seems likely to be a documentation fault, I have raised this internally.

The fix for the default I2C address issue is now pushed to master here in commit fdc78b8: I will wait to delete the preview branch until some time next week, just in case anyone has begun using it.

from ubxlib.

djfurie avatar djfurie commented on September 23, 2024

As another data point, I see some of the expected results when I completely power cycle my board. If I simply try to reset the module without the power cycle it's failing as above...

Output after a cold reset:

U_GNSS: sent command b5 62 0a 06 00 00 10 3a.
U_GNSS: decoded UBX response 0x0a 0x06: 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 [body 120 byte(s)].
U_GNSS: sent command b5 62 06 04 04 00 00 00 09 00 17 76.
U_GNSS: sent command b5 62 0a 06 00 00 10 3a.

Note that I'm not seeing a response to the second MSGPP command.

Output after a warm-reset:

U_GNSS: sent command b5 62 0a 06 00 00 10 3a.

No response even to the first command.

from ubxlib.

RobMeades avatar RobMeades commented on September 23, 2024

Very strange. FYI, in our ubxlib regression test environment, for consistency of testing, all GNSS devices have their RESET_N line connected to the MCU board that they are working with and that line will be pulled low for 500 ms then released and we wait for at least 2 seconds (probably a lot longer than that as we run the port tests first) before we move on with testing.

It would be interesting to know if, in the fail cases, you are getting that 0xFF 0xFF response to the read from register 0xFD and, when that happens, what does the MAX-M10S actually send when the ubxlib code tries to read the stuff that it thinks is queued. Of course, we don't monitor the I2C lines directly during a ubxlib regression test but there is also no retrying going on anywhere so if it failed we would see it, on the other hand if it said 0xFF 0xFF for a little while then switched to something sensible within the message response timeout we wouldn't notice either.

from ubxlib.

ValterMinute avatar ValterMinute commented on September 23, 2024

I have an issue with a similar configuration (zephyr, MIA-M10Q and NRF52832 connected via I2C):
#199
Those may be completely different problems, but I am linking it here, in case a similar setup may help understanding the issue.

from ubxlib.

ValterMinute avatar ValterMinute commented on September 23, 2024

@djfurie sorry, but I checked this issue before posting mine and in the meantime you seem to report exactly what I see in my own setup...

from ubxlib.

Naquino14 avatar Naquino14 commented on September 23, 2024

Looking at your device tree stuff, I notice you have:

   max0: maxm10s@42 {
        status = "okay";
        compatible = "u-blox,maxm10s";
        reg = <0x42>;
    };

...in the I2C entry, which I guess is for another driver inside Zephyr. Is it possible that driver is somehow still trying to do something, might somehow be active? Not that it should cause i2c_transfer() to return an error, of course, just thought I'd point it out.

Aaron and I were working on this together. When I removed it (at the time I was also only calling uDeviceOpen in init_maxm10s per the requirements of /port/platform/zephyr/Readme.md) uDeviceOpen returned -2. It would be helpful to test again though, so I will pull Aaron's changes and delete that node in the tree and let you know what I find.

Hi just following up. Sorry for the radio silence, Aaron has been busy with work and I have been busy with classes. I have updated my branch in our flight software repo to start testing y'alls suggestions.

@RobMeades Deleting the max0 node in the devicetree has no effect, and uDeviceOpen still returns -8.


@djfurie I have tried your suggestion here so my max0 node now looks like:

    max0: maxm10s@42 {
        status = "okay";
        compatible = "u-blox,maxm10s";
        reg = <0x42>;
        i2c-address = <0x42>;
        transport-type = "i2c1";
        module-type = "U_GNSS_MODULE_TYPE_M10";
        i2c-already-open;
    };

Unfortunately, uDeviceOpen` still returns -8. If you suspect its a hardware issue I can ask my team to see if I can share the schematics of our custom board, and link our hardware lead to this issue.

from ubxlib.

Naquino14 avatar Naquino14 commented on September 23, 2024

Very strange. FYI, in our ubxlib regression test environment, for consistency of testing, all GNSS devices have their RESET_N line connected to the MCU board that they are working with and that line will be pulled low for 500 ms then released and we wait for at least 2 seconds (probably a lot longer than that as we run the port tests first) before we move on with testing.

It would be interesting to know if, in the fail cases, you are getting that 0xFF 0xFF response to the read from register 0xFD and, when that happens, what does the MAX-M10S actually send when the ubxlib code tries to read the stuff that it thinks is queued. Of course, we don't monitor the I2C lines directly during a ubxlib regression test but there is also no retrying going on anywhere so if it failed we would see it, on the other hand if it said 0xFF 0xFF for a little while then switched to something sensible within the message response timeout we wouldn't notice either.

Out of curiosity, I checked in our schematic if we had this pin connected, and we do. I will attempt to make a node for this pin and reset the chip before calling uDeviceOpen.

from ubxlib.

RobMeades avatar RobMeades commented on September 23, 2024

Hi again: can you post the debug output printed by ubxlib with i2c-address = <0x42> set?

EDIT: assuming it is unchanged from what you originally posted above, the only place in the ubxlib code where I think U_ERROR_COMMON_PLATFORM (-8) would be returned is here:

handleOrErrorCode = (int32_t) U_ERROR_COMMON_PLATFORM;

...if it can't find your i2c1 binding, and the only reason I can see that it would do that is if CONFIG_I2C is not defined for the build (which is what brings in the Zephyr I2C code) and, indeed, looking at what is posted originally above, it doesn't seem to be...?

from ubxlib.

RobMeades avatar RobMeades commented on September 23, 2024

Thanks: so it seems likely we are in uGnssPwrOn() and the error code we are seeing has been set just here:

errorCode = (int32_t) U_ERROR_COMMON_PLATFORM;

uGnssPrivateSendOnlyCheckStreamUbxMessage() has called uGnssPrivateSendReceiveUbxMessage() to send UBX-MON-MSGPP. The send part of that has worked, or we wouldn't see the U_GNSS: sent command... debug print, so the I2C address is now correct, but it looks like the receiveUbxMessageStream() call a few lines lower down has found no response; from the timestamps we seem to have hit the 10ish second timeout.

This makes the situation look somewhat similar to @djfurie, so it would be interesting to see if resetting the GNSS device a few seconds before you begin would put it into a more receptive (transmittive, I suppose :-)) state. The odd thing is that the GNSS device is responsive, because it has I2C-acked the poll for UBX-MON-MSGPP, it just appears not to be sending back the UBX-MON-MSGPP response for some reason.

At this point a trace of the I2C lines with something like a Saleae probe or equivalent, if you can do so, probably would be the right thing to obtain. That, or you could put in a temporary hack to print out the value of errorCodeOrReceiveSize just here:

if (errorCodeOrReceiveSize == sizeof(buffer)) {

from ubxlib.

RobMeades avatar RobMeades commented on September 23, 2024

EDIT: sorry, not there, one line lower down, after it has been populated with the contents of buffer[]:

errorCodeOrReceiveSize = (int32_t) ((((uint32_t) buffer[0]) << 8) + (uint32_t) buffer[1]);

from ubxlib.

Naquino14 avatar Naquino14 commented on September 23, 2024

Is it possible that the GNSS device is faulty?

from ubxlib.

RobMeades avatar RobMeades commented on September 23, 2024

will carve out some time Today and or Tuesday to debug and find the contents of that buffer on #196 (comment)

If, when you do that, it turns out to contain either 0xFFFF or 0, you could try leaving the GNSS device for, say 10 seconds, after you have reset it, to see if there is some kind of start-up latency that needs to be accounted for.

from ubxlib.

RobMeades avatar RobMeades commented on September 23, 2024

Thanks @cturvey: that's quite a cool feature; whether we could make use of it in this case would depend upon how the HW happens to look in respect of access to the UART HW.

from ubxlib.

cturvey avatar cturvey commented on September 23, 2024

Yes was more of a response to "not wired to test-points" situations.
But yes would require UART1 access to effect.

from ubxlib.

Naquino14 avatar Naquino14 commented on September 23, 2024

It is possible to remap the I2C SCL/SDA to the UART1 RX/TX, either temporarily, or permanently. This can be helpful with some of the off-the-shelf UAV modules which only export the UART1, and a lot of systems want to use I2C due to lack of UARTs on their MCU, or simply preferring the multi-drop, simple cabling of I2C

I2C on UART pins PIO (TX=SCL, RX=SDA)
 0x10510003 CFG-I2C-ENABLED = 1
 0x10510004 CFG-I2C-REMAP = 1
 0x10520005 CFG-UART1-ENABLED = 0
 
Or with UART Enabled but UART/I2C pins swapped (SWAP TX/SCL RX/SDA)
 0x10520005 CFG-UART1-ENABLED = 1  
 0x10520004 CFG-UART1-REMAP = 1
 0x10510003 CFG-I2C-ENABLED = 1
 0x10510004 CFG-I2C-REMAP = 1

https://portal.u-blox.com/s/question/0D52p0000E8WgehCQC/how-to-talk-to-maxm10s-over-i2c https://portal.u-blox.com/s/question/0D52p0000DcUjwwCQC/how-to-check-if-ubxm10050kb-device-contains-flash

@AarC10

from ubxlib.

AarC10 avatar AarC10 commented on September 23, 2024

If, when you do that, it turns out to contain either 0xFFFF or 0, you could try leaving the GNSS device for, say 10 seconds, after you have reset it, to see if there is some kind of start-up latency that needs to be accounted for.

Thank you for the suggestions. I implemented code for resetting the chip. It managed to initialize once, but every other time I reset the entire board, it will fail. Been playing with different sleep values. Currently, we hold the reset pin low for 100 ms, with around a 10ms delay before calling our inits. In the below screenshot, you can see it work the first try, but when I reset the board it goes into a loop trying to reset the GPS chip and re-run initialization (which eventually manages to initialize). However, looping until it works is probably not the most ideal since its non deterministic. Any other suggestions?

Screenshot_20240217_094633

from ubxlib.

RobMeades avatar RobMeades commented on September 23, 2024

Thanks: I think maybe we're focusing on the resetting too much. I only suggested it while we are trying to debug what appears to be an I2C issue without having access to the I2C lines themselves, just to eliminate any strangeness related to there being a load of data inside the MAX-M10S to be got out of the way before the wanted data turns up.

For the purposes of our experiment, I would recommend:

(a) resetting the GNSS device, as you have done, waiting a couple of seconds to be quite sure the GNSS device is booted etc., then see whether you get responses to the I2C commands that ubxlib sends at device open.

(b) printing out the value of errorCodeOrReceiveSize just after this line:

errorCodeOrReceiveSize = (int32_t) ((((uint32_t) buffer[0]) << 8) + (uint32_t) buffer[1]);

(c) printing out the return value of returned by i2c_transfer() just here:

if (i2c_transfer(pDevice, &message, 1, address) == 0) {

It might be that the return values will tell us something.

If not, then I can only conclude that there is something going wrong on the I2C HW lines - adequate pull ups, grounding, line length, that kind of thing. We happily use GNSS devices with 10 cm of flying lead to an MCU with (I think) 1k pull-up resistors but we do know that some of the STM32 MCUs are less reliable than others: in particular, under some circumstances some STM32 MCUs don't work well with I2C at 100 kHz clock:

https://www.st.com/resource/en/errata_sheet/es0206-stm32f427437-and-stm32f429439-line-limitations-stmicroelectronics.pdf

...so we usually run at 400 kHz, just in case (of course I can't say that this is in any way related to your issue).

from ubxlib.

Naquino14 avatar Naquino14 commented on September 23, 2024

I can confirm we are running I2C at 400 kHz, and have 10k Ω pullups on our lines.
cc @bch2857

from ubxlib.

RobMeades avatar RobMeades commented on September 23, 2024

10 k might not be enough: I'm pretty sure we use 1.2k.

from ubxlib.

Naquino14 avatar Naquino14 commented on September 23, 2024

I modified the i2c-clock-hertz entry to 400 kHz and looks like that might have done the trick!
image

from ubxlib.

RobMeades avatar RobMeades commented on September 23, 2024

Interesting! Don't want to hex it but how many times have you tried?

from ubxlib.

RobMeades avatar RobMeades commented on September 23, 2024

Is it possible to swap the 10k resistors for something smaller?

from ubxlib.

RobMeades avatar RobMeades commented on September 23, 2024

Actually, if you could do that, you could probably solder some test leads to the non-ground end...?

from ubxlib.

Naquino14 avatar Naquino14 commented on September 23, 2024

Unfortunately everything is SMD and i'm not familiar with the board layout. Our hardware team would have to make that modification for us.

from ubxlib.

Naquino14 avatar Naquino14 commented on September 23, 2024

I do have a similar ST MCU board and a sparkfun breakout of the MAX-M10s chip. I could talk to Aaron about setting it up.

from ubxlib.

Naquino14 avatar Naquino14 commented on September 23, 2024

Im still stepping through the program to get to that line... So no updates for that yet.
My hardware lead told me that he would be able to swap out the SMD resistors for me to 1.2k sometime Monday since he has the proper equipment.

from ubxlib.

RobMeades avatar RobMeades commented on September 23, 2024

Suggest we start with just attaching test leads to the I2C lines so that we can monitor them; change one thing at a time. Do you have a Saleae probe or equivalent?

from ubxlib.

Naquino14 avatar Naquino14 commented on September 23, 2024

I have a Saleae equivalent that works with Logic 2, however I don't have test probes small enough for the job. I could get some purchased now but they would arrive on Tuesday.

from ubxlib.

RobMeades avatar RobMeades commented on September 23, 2024

Get your HW man to solder some wires to the I2C lines and that should solve your probe problem...?

from ubxlib.

Naquino14 avatar Naquino14 commented on September 23, 2024

Sure, let me ask him.

from ubxlib.

Naquino14 avatar Naquino14 commented on September 23, 2024

@RobMeades How exactly do I log information within the ubxlib source code?

from ubxlib.

RobMeades avatar RobMeades commented on September 23, 2024

How exactly do I log information within the ubxlib source code?

printf().

from ubxlib.

Naquino14 avatar Naquino14 commented on September 23, 2024

Good evening. We managed to solder in test lines on our i2c pins, and I also added printf statements on the lines you asked.
Here is the serial output:
image

And here is part of the I2C transactions in Logic 2 (beginning, middle, end):
image
image
image

from ubxlib.

RobMeades avatar RobMeades commented on September 23, 2024

Could you attach the .sal file (maybe ZIPped)?

from ubxlib.

RobMeades avatar RobMeades commented on September 23, 2024

Also, does your Saleae version have analog capture? Would be interesting to see that I think, since what you have above makes it really look like nothing at all is coming back from the GNSS chip, ever.

from ubxlib.

RobMeades avatar RobMeades commented on September 23, 2024

If you don't have analogue measurement capability, this is probably the right time for @bch2857 to monitor the SDA/SCL lines with a 'scope while the code is running and trying to communicate with the GNSS device, see what they look like, since 0xFF looks like either the GNSS device is not able to clock out any data or the MCU is not able to clock out any data that the GNSS chip is able to clock in, IYSWIM.

If you are willing to make a speculative change (given you're using such tiny components it might damage your board of course) then maybe now is the time to try swapping your 10k pull-ups for, say, 4.7k pull-ups.

from ubxlib.

Naquino14 avatar Naquino14 commented on September 23, 2024

GitHub didn't let me upload my data so here is a link from my personal fileserver.
Our school might have some logic analyzers with analog capability, but I cant say for sure.
Would an oscilloscope work?

from ubxlib.

RobMeades avatar RobMeades commented on September 23, 2024

Thanks, got the file, but looking at what you pasted in again more closely, there is something strange in it.

Here is the start of a working I2C exchange I happened to have already captured:

image

Here is the same from your picture above:

image

The Saleae seems to be detecting a stop bit after the write of address 0xFD; that stop bit should not be there: you can see from the code that the last parameter to uPortI2cControllerSend() is true, meaning noStop. You can see where the implementation of uPortI2cControllerSend() for Zephyr only includes I2C_MSG_STOP in the list of flags if noStop is false. I wonder if there is something weird about the STM32 I2C driver implementation for Zephyr and its capability to NOT send a stop bit?

Before you do anything potentially damaging with the HW, let me browse their code; will respond probably tomorrow now.

Would an oscilloscope work?

A storage scope, yes: properly triggered it should give you a sufficient view.

from ubxlib.

Naquino14 avatar Naquino14 commented on September 23, 2024

Awesome, thank you.
The zephyr implementation being weird could be a possibility, considering a while ago before we ran zephyr on our hardware I wrote a driver to interface with the MAX which worked without issues.

from ubxlib.

djfurie avatar djfurie commented on September 23, 2024

@djfurie: what MCU are you using? If the "P" in an orange circle in your logic analyzer's trace means "stop bit" you may have the same issue.

STM32WL =)

from ubxlib.

RobMeades avatar RobMeades commented on September 23, 2024

@djfurie: ah. There we are then. Ugh.

from ubxlib.

RobMeades avatar RobMeades commented on September 23, 2024

A quick fix would be to comment out this line...

Actually, it may not be quite that simple: in our native STM32Cube driver code we also need to remember to ignore the busy flag on the next transaction as the HW is "stop bit aware", so something equivalent probably needs to be done in the Zephyr STM32 I2C driver also.

from ubxlib.

AarC10 avatar AarC10 commented on September 23, 2024

Thanks for taking the time to help us sort it out. We are very grateful for it. So if we're stuck with Zephyr's current I2C implementation, could we try using the UART that is connected to the chip instead? IIRC, we were also having issues trying to initialize over UART as well. I could need to take a closer look though when I have more time after work or over the weekend or I can delegate to @Naquino14

from ubxlib.

RobMeades avatar RobMeades commented on September 23, 2024

Yes, you can use UART or SPI instead of I2C, up to you really.

from ubxlib.

RobMeades avatar RobMeades commented on September 23, 2024

Summarising the discussion at zephyrproject-rtos/zephyr#69290, there is ambiguity in the way the Zephyr I2C API is meant to operate.

On the face of it, the application can choose, for any I2C transaction, whether that transaction has a stop-bit or not. This is how the Nordic I2C driver works. The STM32 driver takes a different view: every I2C transaction must end with a stop bit; the [potentially multiple] I2C exchanges (sends/receives) within each transaction may omit a stop-bit at each exchange, but at the end of an I2C transaction the STM32 I2C driver enforces a stop-bit; the application has no control over that.

It is the Zephyr I2C maintainer's view that the STM32 driver is more correct, since there are apparently mutexes within at least some of the Zephyr I2C drivers (I guess the STM32 driver has them) to ensure coherent access. He is going to update the Zephyr API documentation to explain how it is meant to work.

All of which means that, right now, STM32-under-Zephyr won't work with ubxlib and GNSS over I2C.

What we will need to do is create a new ubxlib I2C uPort API function, which I'm thinking will probably be something like this:

int32_t uPortI2cControllerExchange(int32_t handle, uint16_t address,
                                   const char *pSend, size_t bytesToSend,
                                   char *pReceive, size_t bytesToReceive,
                                   bool noInterveningStop);

This will replace both uPortI2cControllerSend() and uPortI2cControllerSendReceive(). We will need to implement this on all platforms but we can begin by providing a weakly-linked implementation which just calls uPortI2cControllerSend() and uPortI2cControllerSendReceive() under the hood; we have to do this in any case as there may be customer implementations of the existing ubxlib I2C uPort API which we do not want to break. The implementation of uPortI2cControllerExchange() for Zephyr, though, will do send and receive in a single transaction and then STM32-I2C-under-Zephyr will begin working.

I won't have an STM32 board to hand until Monday now but I can create all of the code and get it working in weakly-linked-call-through mode in advance of that. Likely I will be able to make a preview branch of the intended solution available here by mid next week for you to test. Note that there are apparently two STM32 I2C driver implementations within Zephyr (for different versions of their MCUs); given the variability of this thing, doing your own testing will be pretty essential I think.

Questions/comments welcomed.

from ubxlib.

Naquino14 avatar Naquino14 commented on September 23, 2024

Good afternoon. After switching to UART, I am getting -8 from uDeviceOpen:
image
Im working on getting my logic analyzer plugged in at the moment.
Here is the new cfg-device-gnss node in the dts:

/{
    cfg-device-gnss {
        status = "okay";
        compatible = "u-blox,ubxlib-device-gnss";
        transport-type = "usart2";
        module-type = "U_GNSS_MODULE_TYPE_M10";
    };
}
// ...
&usart2 {
    pinctrl-0 = <&usart2_tx_pa2 &usart2_rx_pa3>;
    pinctrl-names = "default";
    current-speed = <9600>;
    status = "okay";

    //    ubxlib-usart2 {
    //        status = "okay";
    //        current-speed = <9600>;
    //    };
};

u-blox,ubxlib-device-gnss.yaml:

# ...
    uart-baud-rate:
        type: int
        default: 9600

modified init method:

int init_maxm10s(gnss_dev_t *dev) {
    LOG_INF("Resetting MAX-M10S GNSS Module\n");
    gpio_pin_set_dt(&ubx_rst, GPIO_ACTIVE_LOW);
    k_msleep(500);
    gpio_pin_set_dt(&ubx_rst, GPIO_ACTIVE_HIGH);
    k_msleep(2000);

    int ret = uPortInit();
    if (ret != 0) {
        LOG_ERR("uPortInit() returned %d\n", ret);
        return ret;
    }

    ret = uPortUartInit();
    if (ret != 0) {
        LOG_ERR("uPortUartInit() returned %d\n", ret);
        return ret;
    }

    ret = uDeviceInit();
    if (ret != 0) {
        LOG_ERR("uPortDeviceInit() returned %d\n", ret);
        return ret;
    }

    ret = uDeviceOpen(NULL, &dev->gnssHandle);
    if (ret != 0) {
        LOG_ERR("uDeviceOpen() returned %d\n", ret);
        return ret;
    }

    return 0;
}

from ubxlib.

Naquino14 avatar Naquino14 commented on September 23, 2024

Update: Im not sure what I did, but the device now consistently initializes.

from ubxlib.

RobMeades avatar RobMeades commented on September 23, 2024

Interesting: -8 is U_ERROR_COMMON_PLATFORM, which I wouldn't have thought would have been something that fixes itself.

I guess you have an alias entry [EDIT corrected link] in your .dts file so that ubxlib see's a uart (which is what it is looking for) rather than a usart (i.e. without the s)? Without that you would get U_ERROR_COMMON_PLATFORM.

from ubxlib.

AarC10 avatar AarC10 commented on September 23, 2024

We did also find a workaround a few days ago for at least reading and parsing the NMEA packets easily. Zephyr released a GNSS subsystem in their latest release a few days ago. You can just give it a UART device and it seems to read the stream, parse the packet and then put everything in a struct in a callback for you to handle.
Screenshot_20240226_225931

from ubxlib.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.