raspberrypi / pico-sdk

BSD 3-Clause "New" or "Revised" License
3.74k stars 933 forks source link

Unable to measure VSYS/ADC3/GPIO29 with WIFI #1222

Closed pspeybro closed 1 year ago

pspeybro commented 1 year ago

I have a setup with Pico W + pimoroni lipo shim. I am trying to run it on batteries, measure some sensors and submit the data via wifi. I am also trying to check battery status from time to time to get an idea when charging is needed. There is a micropython example here, but I found that this also does not work for Pico W.

I also looked on this forum thread where others also struggle with doing this with python in combination with using WIFI. https://forums.raspberrypi.com/viewtopic.php?p=2078340#p2078340 (my username = Darkness)

I am trying to accomplish this using the SDK.

From the datasheet: `GPIO29 OP/IP wireless SPI CLK/ADC mode (ADC3) to measure VSYS/3 GPIO25 OP wireless SPI CS - when high also enables GPIO29 ADC pin to read VSYS WL_GPIO2 IP VBUS sense - high if VBUS is present, else low

Due to pin limitations, some of the wireless interface pins are shared. The CLK is shared with VSYS monitor, so only when there isn’t an SPI transaction in progress can VSYS be read via the ADC. The Infineon CYW43439 DIN/DOUT and IRQ all share one pin on the RP2040. Only when an SPI transaction isn’t in progress is it suitable to check for IRQs. The interface typically runs at 33MHz`

So it is clear that due to pins being shared with the wifi chip, it is not possible to measure VSYS using GPIO29+GPIO25.

This seems like a very common use case, so an example would be very useful for the community.

kilograham commented 1 year ago

cc @dp111

peterharperuk commented 1 year ago

is there a way to know if an SPI transaction is in progress or not to make sure an ADC3 measurement can be done? Is there a reliable way to repeatedly enable and disable wifi to make GPIO29 and GPIO25 available?

Hi. If you're writing C code using pico-sdk, you shouldn't really need to know when a SPI transaction is in progress or disable WIFI. We set the chip select low when we're using SPI to talk to the wifi chip and then set it high again when we're done. If you want to read VSYS then you just need to make sure we don't do any wifi stuff while you're using GPIO29_ADC. You should be able to do this by wrapping your use of GPIO29_ADC with the CYW43_THREAD_ENTER / EXIT macros?

I'm not sure this helps a lot with Micropython - I'll have to think about that, and read the forum thread.

is cyw43_arch_gpio_get(2); the correct way to read WL_GPIO2/VBUS sense?

I can add a #define for the gpio if it helps. (later: edited, you're fine to call this inside CYW43_THREAD_ENTER / EXIT)

This seems like a very common use case, so an example would be very useful for the community

Yes, good point https://github.com/raspberrypi/pico-examples/issues/324

peterharperuk commented 1 year ago

Maybe we should support a rp2.read_vsys method in Micropython?

pspeybro commented 1 year ago

I'm just using C or C++, not micropython. It's just the pimoroni example and forum thread that are using micropython.

I will try with the CYW43_THREAD_ENTER / EXIT. I did not encounter these before. Are they explained somewhere in the datasheet or documentation?

Also, do I need to pull GPIO25 high to read GPIO29_ADC?

An extra #define is not strictly necesary, but because the datasheet refers to WL_GPIO2 and micropython seems to define that, it might be useful if those WL_GPIO1 and WL_GPIO2 are also defined, so that you do not need to guess that these just correspond to 1 and 2.

I was considering making an issue in the example repo, so thank you for doing that. If I get my test working, I will post the code there.

peterharperuk commented 1 year ago

Also, do I need to pull GPIO25 high to read GPIO29_ADC?

I haven't tested it yet but it has to be high (famous last words) while we're not doing anything with wifi or else a spi transaction would be ongoing? You might have to change the function of gpio 29 while you're reading from it and restore it before calling CYW43_THREAD_EXIT.

I did not encounter these before. Are they explained somewhere in the datasheet or documentation?

There's some discussion in the examples of using this for for networking w.r.t. cyw43_arch_lwip_begin. In the latest sdk (soon to be released) there's a new asynchronous api that has an explicit lock function which might be more obvious.

Out of interest, what "method" are you using to build your code? I'm guessing you're linking to pico_cyw43_arch_lwip_threadsafe_background? If you use "poll" you shouldn't even need the lock.

pspeybro commented 1 year ago

Out of interest, what "method" are you using to build your code? I'm guessing you're linking to pico_cyw43_arch_lwip_threadsafe_background? If you use "poll" you shouldn't even need the lock.

Indeed, I am using pico_cyw43_arch_lwip_threadsafe_background, as I was under the impression that polling is generally less efficient, but I did not read up on that in detail yet...

kilograham commented 1 year ago

cyw43_thread_enter() and cyw43_thread_exit() are nominally better (but equivalent to CYW43_THREAD_ENTER and CYW43_THREAD_EXIT

still seems that neither are particularly documented.

pspeybro commented 1 year ago

That cyw43_thread_enter()/cyw43_thread_exit() definitely seems to improve things. (requires #include "pico/cyw43_arch/arch_threadsafe_background.h")

Checking VBUS/WL_GPIO2 seems to correctly indicate whether powered from usb or battery.

Initializing the ADC3 between the enter/exit seems to work as well and no need to set GPIO25 high.

Below is the log from the data submitted via wifi to my api. Capacitive moisture sensor on ADC2, 1-wire temperature sensor, VSYS and VBUS

[2023-37-06 22:37:41] Moisture: 2,491, Temperature: 20,5, Voltage: 4,981, Vbus: 1 <- USB [2023-37-06 22:37:43] Moisture: 2,486, Temperature: 20,5, Voltage: 4,984, Vbus: 1 <- USB [2023-37-06 22:37:45] Moisture: 2,49, Temperature: 20,5, Voltage: 4,123, Vbus: 0 <- Battery [2023-37-06 22:37:46] Moisture: 2,49, Temperature: 20,5, Voltage: 4,123, Vbus: 0 <- Battery [2023-37-06 22:37:48] Moisture: 2,49, Temperature: 20,5, Voltage: 4,126, Vbus: 0 <- Battery [2023-37-06 22:37:50] Moisture: 2,49, Temperature: 20,5, Voltage: 4,984, Vbus: 1 <- USB [2023-37-06 22:37:52] Moisture: 2,489, Temperature: 20,5, Voltage: 4,979, Vbus: 1 <- USB

Vbus is nicely going to 0 when unplugging usb, and the voltage (VSYS/GPIO29_ADC ) measurement goes from 4.98 to 4.12 when switching to battery. This way I can skip reading the VSYS when connected via USB.

We do need to restore the GPIO29_ADC setting back to the wifi settings. After checking the pin settings during wifi and ADC, it seems I need to set the function back to USB and pull down (though the pull down did not seem necessary) gpio_set_function(29, GPIO_FUNC_PIO1); //7 gpio_pull_down(29); //does not seem necessary, but this way it is back to how it was before adc_gpio_init(29);

I will clean up my code a little bit and post it in the example issue

Thank you very much for the hints!

pspeybro commented 1 year ago

Pull request with example code: https://github.com/raspberrypi/pico-examples/pull/326

liaan commented 1 year ago

Hi, Apologies first, I have no knowledge of micropython. But was wondering, can one not have a interrupt Lising on the CS going high (wifi not communicating), Set pin 29 to analogue, read, revert, and then continue?

Not sure how interrupt priorities work with micropython, might be a option?

peterharperuk commented 1 year ago

It's the same on MP except you have to use pendsv_suspend / pendsv_resume instead of cyw43_thread_enter / cyw43_thread_exit. Thus my suggestion that we add a rp2 method to read vsys as reading the gpio from MP won't take the lock. I don't much fancy exposing the lock in MP.

peterharperuk commented 1 year ago

See my attempt https://github.com/raspberrypi/pico-examples/pull/331

ZodiusInfuser commented 1 year ago

Hi All, I just want to chime in here with my support for a proper way to read the vsys voltage on Pico W from Micropython.

At Pimoroni we have the Enviro range of products which have a Pico W Aboard, and we had hoped to use the onboard VSYS measurement to measure the board's battery voltage. And indeed we can do this, but not without causing issues with the WiFi communication down the line. We tried doing the reading when the CS pin was not selected, without success, and currently have a customer PR that reportedly works https://github.com/pimoroni/enviro/pull/146 but I would like much more testing done before merging it in and having it enabled by default.

peterharperuk commented 1 year ago

a proper way to read the vsys voltage on Pico W from Micropython

The Micropython repo is here https://github.com/micropython/micropython

ZodiusInfuser commented 1 year ago

Yes, I am very much aware. You said yourself though that:

Maybe we should support a rp2.read_vsys method in Micropython?

I was showing my support for that.

pspeybro commented 1 year ago

Just a followup to this issue. In my tests with wifi, getting the VSYS value is working nicely without a problem. However, with the new release of the SDK 1.5.0, I thought of trying this while using bluetooth LE instead of wifi and now I seem to get very low values again (almost zero).

I already tried a few things like the cyw43_thread_enter()/cyw43_thread_exit() even though I am using pico_cyw43_arch_none.

Link libraries (cmake): pico_stdlib
hardware_adc # for ADC pico_btstack_ble pico_btstack_cyw43 pico_cyw43_arch_none

Also recompiling my previously working code with wifi using SDK 1.5 is still working. Is there something regarding bluetooth that might prevent getting the VSYS/ADC3 voltage?

Getting the VBUS value (to check if it is charging or on battery is working.

peterharperuk commented 1 year ago

Is there something regarding bluetooth that might prevent getting the VSYS/ADC3 voltage?

I can't think of anything. Are you sure you've accounted for the differences between Pico and Pico W? I can check with my example when I have time https://github.com/raspberrypi/pico-examples/pull/331

tried a few things like the cyw43_thread_enter()/cyw43_thread_exit() even though I am using pico_cyw43_arch_none.

pico_cyw43_arch_none is "threadsafe background" without lwip. You absolutely have to call cyw43_thread_enter()/cyw43_thread_exit if you want to measure vsys on Pico W.

pspeybro commented 1 year ago

I continued from my working example, but I will compare with your example again. Thank you for confirming the need to call cyw43_thread_enter()/cyw43_thread_exit.

I will also check the before and after settings of the gpio to see if I need to restore it to different settings compared to wifi, but also the initial reading is not showing the correct value, so that does not seem to be the cause.

pspeybro commented 1 year ago

After stripping my wifi code and adding the bluetooth parts, it seems to be working now. I still need to compare it with my previous version to see what's different but it seems stable now.

I did have some issues where the voltage measured seemed unstable, starting at 5.02 and then dropping towards 3.5 after 15-20 seconds, even though it was connected with usb. Not sure what caused that, but might have been before I added hci_power_control(HCI_POWER_ON);

peterharperuk commented 1 year ago

I think I see the same behaviour with some instability (I improved this by reading multiple times) and high values on the first read. I'll see if the hw engineer has any thought on why this might be the case.

dp111 commented 1 year ago

It might be worth making sure that after the rising edge of nCS there is a delay of >600ns before the ADC read is started.

On Tue, 7 Mar 2023 at 16:55, Peter Harper @.***> wrote:

I think I see the same behaviour with some instability (I improved this by reading multiple times) and high values on the first read. I'll see if the hw engineer has any thought on why this might be the case.

— Reply to this email directly, view it on GitHub https://github.com/raspberrypi/pico-sdk/issues/1222#issuecomment-1458509018, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEVVFIQV73J67BMXPA6DH2DW25SBJANCNFSM6AAAAAAUSBCKYU . You are receiving this because you were mentioned.Message ID: @.***>

kilograham commented 1 year ago

closing as this seems to be resolved