search

pimoroni / EnviroPlus-FeatherWing

CircuitPython library for the Enviro+ FeatherWing
https://shop.pimoroni.com/products/enviro-plus-featherwing
MIT License
10 stars 11 forks source link

Upgrade library to CircuitPython 5.x #5

Open dglaude opened 4 years ago

dglaude commented 4 years ago

Dear @Pimoroni,

I would like to be notified when your library will be compatible with Circuit Python 5.x (please update this issue), but also I would like to know what are the current compatibility issue.

https://github.com/adafruit/circuitpython/releases/tag/5.0.0

Is it some sensor? Is it DisplayIO?

Regards

wildestpixel commented 4 years ago

This is my frustration also, although I've returned my feather. The library this was launched with, albeit not in the standard folder and .mpy structure seems to be what you are stuck with. I've approached everything separately via ESP8266's at very low cost and through C++ Its just the very nice gas sensor i miss.

Gadgetoid commented 4 years ago

Right now I just don't have the bandwidth to look into this library or the CircuitPython/Feather ecosystem in general - it's a lot of new knowledge and idiosyncrasies to onboard under usual circumstances, but present circumstances and the resulting complications have made it untenable.

It's not our intention for this library to be incompatible, against the grain or abandonware but please bear in mind this is our first foray into the CircuitPython/Feather ecosystem, championed by an intern, and released just shortly before the world turned upside-down.

We'll get this worked out eventually.

Gadgetoid commented 4 years ago

Could anyone summarise the issues with 5.x compatibility for me?

From my understanding with comments in #4 and #6 this library (or collection of libraries as it were) could use some reformatting to be more idiomatic to the CircuitPython ecosystem. It would be helpful to know where it went wrong and what shape it should take.

dglaude commented 4 years ago

I don't know exactly why it is not compatible with 5.x. my guess is some displayio feature as I don't believe I2C or UART did change much from 4.x to 5.x.

I did try, and some simple example did work, while other freezed my Feather like I had never seen before. That is were I gave up a little bit, but I plan to dig further to isolate the problem.

The problem with 4.x is that newer board only have support for 5.x and if you are serious about using BLE, then you want the lattest. Also, some I2C functions have an alternative and the old equivalent will be removed in 6.x.

I think it would be interesting to discuss with Adafruit core developper such as Scott Shawcroft @tannewt maybe joining their Discord server or do a conversation in their weekly public meeting (Monday's CircuitPython Weekly meeting 11am Pacific / 2pm Eastern) There is an "in the weed" section for open discussion on any topic.

I did try to undestand how all that software work together, and I don't plan to send my FeatherWing back to Pimoroni. But I don't know how much effort I can put into this.

In addition to sensor and display driver supported by Adafruit Driver, Enviro+ use:

wildestpixel commented 4 years ago

I had all of the examples working fine with no hangs on 5.3 both built myself uf2 and download uf2. I hadn’t tried with 5.4.

I don’t think CP4.x vs 5.x is the issue to be fair.

Each of the libs would be best posed in standard compiled .mpy files or at worst single uncompressed .py files making for simple use a la the main CP lib and example bundles, following the same CP / Blinka pin assignations of Dxx and Axx.

I know in building CP 5.x - if you catch it wrong when you sync / refresh your repo your build won't work and send your M4 into safemode. Clean and resync next day seeing new commits relevant and all good.

My factor in difference to yours is me using a Microchip M4 and you're using a NRF52840 is that right ? I only have STM32F405 and M4 (and the STM boards are not flagged in the compatibility for Enviroplus feather , although they will work with the LCD now through displayio, UART is probably going to be the killer for it)

tannewt commented 4 years ago

I don't know exactly why it is not compatible with 5.x. my guess is some displayio feature as I don't believe I2C or UART did change much from 4.x to 5.x.

What example code doesn't work on 5.x? Please be more specific about what doesn't work.

wildestpixel commented 4 years ago

Hi Scott - it does work on 5.x - in fact I've never used with 4.x. Problems must lie elsewhere. I don't have an answer as to whether user is on NRF52840 feather, but I've successfully run the examples on 5.3, and might have run on 5.4 - I can't remember, but don't have the enviroplus feather any longer. All of my success on M4 feather express (you and heirophect made commits after my buiilds of CP for this board were failing recently, and I was able to confirm successful builds again)

David - do you have REPL output at the point any of the code hangs ?

dglaude commented 4 years ago

I don't know what occured last time I tried, let's blame it on the cable or my laptop mu editor or windows.

This time I tested from Linux to recover everything from the repository. I tested on an nRF52840 Express First checking quickly with the last 4.x without problem. Then with 5.4.0-beta.1 and very few test misbehaved (see below).

A bit on the test condition:

print("^^^^^^^^^^^^^^")
#import bme280_simple # Ok 5.4.0-beta.1
#import microphone # Ok 5.4.0-beta.1
#import plotter_bme280 # Ok 5.4.0-beta.1
#import plotter_particulate # ?? 5.4.0-beta.1

#Crash on ChecksumMismatchError after a bit of time (using interval = 1)
#Traceback (most recent call last):
#  File "main.py", line 5, in <module>
#  File "plotter_particulate.py", line 52, in <module>
#  File "/lib/pimoroni_pms5003/__init__.py", line 164, in read
#ChecksumMismatchError: PMS5003 Checksum Mismatch 642 != 28

#import plotter_gas # Ok 5.4.0-beta.1
#import plotter_light_sound # Ok 5.4.0-beta.1
#import plotters_combined # Ok 5.4.0-beta.1
#import proximity_and_light # Ok 5.4.0-beta.1
#import test_all # ?? 5.4.0-beta.1 gases fail with out of expected range

#These tests failed:
#Oxidising gases reading out of expected range
#Reducing gases reading out of expected range
#NH3 gases reading out of expected range

#import screen # Ok 5.4.0-beta.1
#import particulate_sensor # Ok 5.4.0-beta.1
dglaude commented 4 years ago

I made a quick and dirty work around for the ChecksumMismatchError of PMS5003 by trying to read a second time: https://github.com/pimoroni/pms5003-circuitpython/issues/3

dglaude commented 4 years ago

@wildestpixel I have worked around the lack of support for STM.

The biggest issue is with physical_feather_pins, that mysterious physical pin mapping. One by one I found the needed mapping, and then added also the pin not in use by the Enviro+ to be complete: https://github.com/pimoroni/physical_feather_pins/pull/3

The next problem relate to the fact that RTC is not implemented in CP for STM32. So the following two library fail:

Those implement timeout active loop using time.time(). I just replaced that by time.monotonic() that is sufficient for that use. Maybe I should use time.monotonic_ms().

I do not care if monotonic() is missing from Python2 or monotonic_ms() only found in CP. It works for me. But @pimoroni might want to implement it a better way.

I don't have a Feather M4 to try and Feather M0 is very limited in memory and cannot drive the display anymore.

dglaude commented 3 years ago

Maybe the focus should now be on CircuitPython 6.0 but if 5.x is still a target, it gets very stable is in not moving much anymore since 6.0 in alpha. So getting things to work on 5.3.1 is a good idea. But I am on the edge...

I made some progress but now on the @adafruit side. I was unhappy with the result I had, so I looked around at both Adafruit and Pimoroni support for the PMS5003.

My "ChecksumMismatchError: PMS5003 Checksum Mismatch 642 != 28" do seems to be a general problem for my hardware... especially is the cable is not secured properly. I don't care about perfect reading every seconds, I care about having at least one result after a few attempt and not to crash on an exception. So for me the reliability and recovering from bad checksum, bad signature, ... is important.

I made the following change to Adafruit PM25 library and it works for me: https://github.com/adafruit/Adafruit_CircuitPython_PM25/pull/6

Should there be two libraries that support the PMS5003 in Circuit Python, one by Adafruit that make Circuit Python software and sell two particulate product, the serial and an I2C version and the Pimoroni version that is used both by the pHAT and the FeatherWing.

Gadgetoid commented 3 years ago

Probably preaching to the choir here- but it looks like adafruit_pm25.mpy is present in the Adafruit CircuitPython Bundle, so I'm definitely onboard with switching over to that and focussing any efforts to make it better.

The PM25 library only ever seems to look for the first byte of the SOF sequence (0x42)- I wonder if that has any bearing on its reliability. I need to get tooled up to test this, but I've still got other backlog to push out first.

dglaude commented 3 years ago

I wanted to make some glue code to provide Pimoroni API on top of Adafruit library, but maybe it is not needed and I can just update the examples.

The Adafruit library check both signature bytes, the length and the checksum. But that is the common code between I2C and UART.

The UART specific code make less checking, but that is OK, as it will be checked again later.

What I added in a PR is a more aggressive resync by trying to have the first signature byte until the serial queue is empty. This solve my problem when that as soon as I had a bad reading, the lib would never recover because it was only reading one byte at each call.

On Adafruit side, kattni is working on the learn guide and library, so I guess she will review this PR. But feel free to add comment on the PR if you can test it.

Please tell me if I can help on this side.

Gadgetoid commented 3 years ago

It looks like, in lieu of physical_feather_pins the correct approach would be to have all individual drivers accept normal pin definitions on instantiation, and either clearly document in the examples which pins to use on which board, or create separate directories of examples for different boards.

This makes my TODO list something like the following:

tannewt commented 3 years ago

If it works on 5.3.1, then I'd expect it to work in 6.0.0. The changes between the two are minor and unlikely what you are using.

dglaude commented 3 years ago

In addition to example using pimoroni_physical_feather_pins, there is also some use in the screen part of the library and the gas helper:

screen/init.py

command=pimoroni_physical_feather_pins.pin19()
chip_select=pimoroni_physical_feather_pins.pin20()
reset=pimoroni_physical_feather_pins.pin21()

Either you have to keep the pimoroni_physical, or you can have those stuff passed as parameter by the caller. The goal is to be able to keep this as an mpy so put the variable outside of this.

gas.py

#enable = digitalio.DigitalInOut(board.A4)
enable_pin = digitalio.DigitalInOut(pimoroni_physical_feather_pins.pin9())
# OX = analogio.AnalogIn(board.A2)
OX = analogio.AnalogIn(pimoroni_physical_feather_pins.pin7())
# RED = analogio.AnalogIn(board.A1)
RED = analogio.AnalogIn(pimoroni_physical_feather_pins.pin6())
# NH3 = analogio.AnalogIn(board.A0)
NH3 = analogio.AnalogIn(pimoroni_physical_feather_pins.pin5())

(to be verified) I guess all the board have A0 .. A4 at the same spot...

Or this mapping must be external to the library to be able to use an mpy (if that gas sensor has a name, maybe this is a driver?

Or you keep using pimoroni_physical.

Gadgetoid commented 3 years ago

Actually a good idea to make a driver for the gas sensor- on Pi we have to hook it up through an ADS1015 ADC so the whole setup becomes.... not really a specific device to target. But here we're connected directly to the Feather's ADC pins so we can set up a driver that accepts a collection of pins and has properties (since that appears to be the done thing) to access the caculated gas values. The result would be readily portable to other uses of the same sensor.

I will add that to the TODO list.

Edit: Gas sensor driver is under way - https://github.com/pimoroni/Pimoroni_CircuitPython_MICS6814

Edit: Gas sensor API refined slightly and tweaked, should ... work... need to test!

Edit: Gas sensor tested somewhat and item ticked off TODO list. The LTR559 library is also now in the community bundle.

dglaude commented 3 years ago

@Gadgetoid my PR on PM25 is now integrated, so you may want to look if you have some improvement you can bring. There is a "UART passive" alternative to the normal UART that is under work. I'll try to setup everything with the latest version on an M4 and see what still need to be done.

dglaude commented 3 years ago

@Gadgetoid I have been looking at the state of play and tried to break down in more detail the change possible. Maybe you want to have a look at this: https://gist.github.com/dglaude/fc66d29f250ecbae7b777a27ec65ef45

Here are my initial attempt:

dglaude commented 3 years ago

I made a PR to adafruit PM25 with 3 out of the 4 improvement you did based on Pimoroni library: https://github.com/adafruit/Adafruit_CircuitPython_PM25/pull/12

Bogdan50 commented 3 years ago

I just got an Enviro+ with a Feather 4 Express partially running with CircuitPython v6. Do I have to switch to v5.3 or can someone explain what changes are necessary to make the code work? The test_all.py succeeds with the BME280 and sound tests only, I had to comment out the LTR559 test.

Gadgetoid commented 2 years ago

I believe we're good up to 6.2.0 now, but I'm not sure what the state of 7.x.x might be.