-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Moving Adafruit_CircuitPython_BusDevice to core #3612
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for doing this! I did a preliminary look, even though this is a draft.
BTW, If your editor has the features or a plug-in, set it up to make sure there's a newline at the end of the file, and also to delete trailing whitespace on each line, either via a command or on save (more convenient). Emacs can do this automatically, and I've seen that some editors can do this too. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! I have some areas of improvement to suggest.
I have a question about the import equivalency of moving this to the core (I2CDevice and SPIDevice are equivalent to their python originals). In the python library a user may use: To call it now they would use:
As the spi_device (and i2c_device) .py files no longer exist is there any problem with this? I got these examples from the BME280 library so was thinking the libraries would have to be updated to use the new core module if that's required. Also wondering how we deal with the case if we cannot include busdevice in the core of the M0 boards you don't want a library to fail. This may be easier to find time to talk about on discord so feel free to ping me there to set up a time. |
Why not just name the core library |
Do you mean |
Yes I do. The last commits I made were to rename everything so it should "just work". But have only tested it with one library (BME280) so far. |
I tested this successfully with Feather Sense using some of the on-board sensors: accel_gyro = LSM6DS(board.I2C())
mag = LIS3MDL(board.I2C()) I took Installed the uf2 built from this PR and it's working properly to read values from both sensors. 🎉 This is awesome! Thank you for working on it @gamblor21 |
Also tested this successfully with MONSTER M4SK build. Deleted adafruit_bus_device from lib folder and I am still able to use the onboard seesaw and the lis3dh accelerometer. Everything is working well. |
I tested it successfully with STM32F405 and an MPU-6050 as well. No bus_device in lib folder, am still able to successfully read values from the sensor. |
@gamblor21 Want to mark this as ready? |
Question 1: Should I make all the M0 small builds exclude this? (Basically all the ones that are failing) Question 2: not 100% sure if I should be running "RUN_BACKGROUND_TASKS and mp_handle_pending" during the I2C enter loop. When I did try it once it was very slow. Not sure why. I could experiment some more too. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question 1: Should I make all the M0 small builds exclude this? (Basically all the ones that are failing)
Yes, for most of them. Just keep an eye out for the ones that freeze the python version in (like CPX). In those, you should be able to remove the frozen version in favor of the native one.
Question 2: not 100% sure if I should be running "RUN_BACKGROUND_TASKS and mp_handle_pending" during the I2C enter loop. When I did try it once it was very slow. Not sure why. I could experiment some more too.
See my suggestion. It tries to get the lock once before checking for background work. You could do mp_handle_pending after the loop if success is false.
I did one more test this morning with Metro ESP32-S2 and a SSD1306 I2C screen. It's working well with those also. No bus_device in lib folder, still able to use the display. |
@gamblor21 just a reminder... the frozen module should be removed from the feather_m0_rfm69 and feather_m0_rfm9x builds. |
Ah thanks, I knew I missed something else. I'll do a search for all the frozen modules to make sure I get them all the next time. |
Where is this at? |
There was one question I asked Dan about an optimization, but otherwise as far as I am aware it is ready (minus the couple conflicts I now see which should be easy to fix). |
Thanks! I replied to that comment. I think it's ready to go once it's updated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, one last thing. I think you'll want to enable BUSDEVICE explicitly on boards that had it frozen in but FULL_BUILD is 0. Otherwise they'll lose it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! Looks great!
Moving the Bus Device library into the core. This is a direct translation of the python into C to hopefully achieve some speedup and potential size reduction.