Use typeshed stubs for more complete static analysis of CircuitPython code #304
Conversation
|
Mikefly123
left a comment
Mikefly123
left a comment
There was a problem hiding this comment.
Great work here Nate! Overall I think it is good to go, just two things that we should resolve before getting the rubber stamp:
- It seems like there is a file from #301 that doesn't need to be in this PR
- We have to make a decision (probably outside of this PR context) on how we will address the long running RP2350
alarmmodule issue so we can update the firmware for V5x boards to get upstream CircuitPython patches
There was a problem hiding this comment.
Are these changes supposed to be in this PR? It looks like they are crossing over from #301
There was a problem hiding this comment.
Haha I wouldn't be surprised if something leaked in but this is supposed to be here. Some of the old detumble code is still part of the codebase!
| now = time.localtime() | ||
| asctime = f"{now.tm_year}-{now.tm_mon:02d}-{now.tm_mday:02d} {now.tm_hour:02d}:{now.tm_min:02d}:{now.tm_sec:02d}" | ||
| now = time.localtime() # type: ignore # PR: https://github.com/adafruit/circuitpython/pull/10603 | ||
| asctime = f"{now.tm_year}-{now.tm_mon:02d}-{now.tm_mday:02d} {now.tm_hour:02d}:{now.tm_min:02d}:{now.tm_sec:02d}" # type: ignore # PR: https://github.com/adafruit/circuitpython/pull/10603 |
There was a problem hiding this comment.
This is great that upstream will soon get this patch!
I made a new issue ticket on the FC board repo to remind me to update all of our firmware files to get this upstream patch. @nateinaction remind me when a version is cut using that patch if you can.
I do want to note though that although V4x boards should be able to automatically update to the latest CircuitPython version V5 boards will be slow to get there because there is still no official upstream support for low power modes and we are having to use a weird branch / fork / open PR to get them with the current firmware.
I don't think this will be a blocker for this PR going in, but we will have to circle back to thinking about doing one of a few options regarding low power sleep in a separate context:
- Remain on the bablokb fork and not get additional circuitpython updates for V5x boards for the time being
- Try to update the fork to be in sync with main while preserving the low power mode code
- Remove the use of
alarmfrom thesleep_helperand stop using low power sleeps until it is officially merged upstream - Task someone on the team to sort things out and get something merged upstream ourselves
To be honest I am kind of in the mindset to implement Option 3 for the codebase for the time being. It will hit our power consumption, but the latest numbers seem to indicate we shouldn't be in trouble (assuming the solar panels are working as intended) and removing the use of deep_sleep and light_sleep will greatly simplify the end user experience with our reference code.
There was a problem hiding this comment.
Those all sound like good options. 3 sounds like what we might do if we're trying to reduce commitment to pysquared while still shipping a release soon and ramping up on fprime. Good option.
There was a problem hiding this comment.
These changes don't technically require a the board firmware to be updated. It would be good to keep pysquared at the same release as the board firmware but what we really need is the new python stubs that get shipped with each release. We can update those independently but the version skew might not be a good development experience.
Mikefly123
left a comment
Mikefly123
left a comment
There was a problem hiding this comment.
Alright this LGTM! Let's get this in!
Kachunk
Summary
This PR moves from using a
typings/directory to using CircuitPython typeshed stubs. This change provides accurate type hints for CircuitPython, replacing the default Python standard library type hints. The CircuitPython-specific stubs are located in thetypeshed/directory. This allows the typechecker catch compatibility issues with CircuitPython code before running it on a device.However, using these stubs means that type hints in test files also reference CircuitPython types, not the standard Python types available in the test environment. As a workaround, we add pyright ignore comments (e.g.,
# pyright: reportOptionalMemberAccess=false) when necessary at the top of test files to suppress related errors. This workaround isn’t ideal. If you have suggestions for handling this issue more effectively, please share your feedback.This PR also adds 3 type ignore comments and carves our a space in the design guide to allow them. All 3 have been fixed upstream and are awaiting a release:
struct_timeandtime.localtime()type hints adafruit/circuitpython#10603It took a year of learning and refactoring but this finally resolves #27