-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Add stm to docs matrix #2598
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
Add stm to docs matrix #2598
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.
I also built locally, and everything¹ seems to be turning out correctly. Thanks @hierophect!
¹ There appears to be some deeper brokenness going on. Things like frequencyio
are showing up as available on the STM32F4s. They shouldn't be, since its not included in a MINIMAL_BUILD
and I don't see it explicitly turned on anywhere. That is mostly outside of this though, so I'll find a better way to document the issue.
I meant to add: I'll hold off on merging. I'm not a huge fan of sending known-incorrect documentation out. Without a better grasp on what is causing the noted problem, I'm not sure how a long a fix would take. Thoughts on holding this until I (or whomever) gets a handle on the secondary problem? |
Are there similar issues on other ports, or just stm32f4? |
@dhalbert, looking at samples for different ports, I'm not seeing similar issues. I'm pretty confident that its only related to the use of |
OK, I will add a 'changes requested' review to hold off on this until the |
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.
Let's hold off until this is fixed:
@sommersoft sys:
I'm pretty confident that its only related to the use of CIRCUITPY_MINIMAL_BUILD. The minimal build logic was added after docs/shared_bindings_matrix.py was written, so it just doesn't handle it.
@hierophect I think so, if you can get it to work, since then the doc will be correct from the start. |
@sommersoft I've adjusted the shared_bindings_matrix.py file - builds ok for me but let me know if you'd like to see any changes. @jepler could you double check the change I made to |
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.
I built locally, a couple times. Verified that the ifeq ($(MCU_SERIES),F4)
patterns in the STM port's mpconfigport.mk work, should they start to drift. Its all 5x5 that I can see.
I kind of liked the ALWAYS/DEFAULT/MINIMAL approach, but I spend no time adding ports. I'll agree with any approach of those that do. 😄
There are a few cleanup things, but they're mostly my doing from the initial effort. I have no issues handling them myself, as I prepare to loop shared_bindings_matrix.py
into providing circuitpython.org with this info.
Thanks @hierophect for getting the STMs into the docs!
@sommersoft I liked the concept of ALWAYS/DEFAULT/MINIMAL at first, but I realized it was backwards in how it affected ports. Setting everything to off and then adding what you support meant that you had to memorize what was in each "level" and then constantly re-evaluate if you should be moving up to the next. Instead, starting at 100% and having a list of everything you turn off is like having a built in TODO list, one that you continually whittle down at as the port progresses (which is what everyone but me was doing anyway). I posted an example in |
@dhalbert could you give this another look? |
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.
Good explanations in the new comments. I had a few corrections, and some queries.
@dhalbert the raw strings were part of the cleanup I mentioned in my review. Nothing wrong with fixing them here; I just figured I'd clean up my own messes. 😄 |
@dhalbert ok fixes are in and merge completed. I went ahead and deleted SMALL_BUILD completely across the port, atmel simply sets FULL_BUILD=0 now. |
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.
ok, getting really close!
@dhalbert should be all wrapped up for one last review! |
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.
OK, looks great, thanks for all the work on this!
As per title. Removes a number of build flags as redundant and reworks the stm port
mpconfigport.mk
to match. Adjustsshared_bindings_matrix.py
to recognize STM32 family categories. Adds a minor addition to porting.rst to explain how build flags impact porting.Builds locally without issue, and correctly lists modules for all STM32 boards in the support matrix list, including the thunderpack (which enables NVM) and the H7 and F7 (which have restricted modules).
Resolves #2587
Resolves #2458