-
Couldn't load subscription status.
- Fork 8.1k
Devicetree: edtlib: fix possible AttributeError in Property.description #51139
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
Devicetree: edtlib: fix possible AttributeError in Property.description #51139
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.
Thanks for your fix!
We do not accept PRs with merge commit, though -- please fix your branch so it doesn't have the merge commit, and has a linear history of one commit on top of something in upstream instead.
https://www.youtube.com/watch?v=mnVNLM7ynMQ
Patch itself looks good, thanks
|
BTW, dtsh looks amazing! Please consider me officially interested in getting support for this into upstream Zephyr when you feel it has matured enough :). cc @gmarull |
dcdca66 to
f47a3cd
Compare
|
@mbolivar-nordic Thanks for the review, and the link: I also prefer a linear git history, but wasn't sure of your policy regarding force push, now I know. I should have red the contribution guidelines more closely, though. Sorry for this. [Edit: |
9384b43 to
4a611e0
Compare
Attempting to access the property Property.description when Property.spec.description is None would raise AttributeError: 'NoneType' object has no attribute 'strip'. Known properties that may not have a description (Property.spec.description is None): - 'compatible' for nodes such as / /soc /soc/timer@e000e010 /leds /pwmleds - 'reg' for nodes such as /soc/timer@e000e010 - 'status' for nodes such as /soc/timer@e000e010 - 'gpios' for nodes such as /leds/led_0 /buttons/button_0 - 'pwms' for nodes such as /pwmleds/pwm_led_0 This patch checks the PropertySpec.description attribute before calling strip(): will return None, and not raise AttributeError. Signed-off-by: Chris Duf <[email protected]> Co-authored-by: Gerard Marull-Paretas <[email protected]>
4a611e0 to
42d321d
Compare
You may find this easier than just heading straight to the source code: https://docs.zephyrproject.org/latest/develop/west/extensions.html#adding-a-west-extension |
Yes, sure, thanks. OTOH, considering my weak Python-Fu, reading some good source code won't hurt ;-) |
@mbolivar-nordic, thanks for the kind words.
At this point (or even before), I'd like to have as many bug reports as possible, for as many different configurations as possible: is it appropriate to post an humble announce to Also at this point, I may open a draft PR to start working on the Thanks. -- |
Yep, of course! I would recommend sending email to
Yep, a draft PR is fine. Here is a (short!) talk which gives an overview of the upstreaming process: https://www.youtube.com/watch?v=mnVNLM7ynMQ |
@mbolivar-nordic , sorry for the delay, I've been overwhelmed at work, then I needed some rest a little farther from keyboards. And apart from a couple updates, mostly to cope with a few edtlib API changes, dtsh did not really mature, until I eventually start to work on the I've updated design and implementation with some lessons learnt from the prototype, and tried to come to a sizeable PR that still implements enough to prove tasteful or useful: #59863. BTW:
Kind regards, |
Attempting to access the Python property
Property.descriptionwhenProperty.spec.descriptionisNonewould raise:Known DT properties that may not have a description (
Property.spec.descriptionisNone):compatiblefor nodes such as/,/soc,/soc/timer@e000e010,/ledsor/pwmledsregfor nodes such as/soc/timer@e000e010statusfor nodes such as/soc/timer@e000e010gpiosfor nodes such as/leds/led_0or/buttons/button_0pwmsfor nodes such as/pwmleds/pwm_led_0This patch checks the
PropertySpec.descriptionattribute before callingstrip(): will returnNone, and not raiseAttributeError.NOTE: I'm not aware of any legit
edtlibuse (e.g. by scripts in zephyr/scripts) that may trigger this issue, in particular I've never faced thisAttributeErrorwhen building a Zephyr application. My use-case is slightly different (a shell-like interface to a devicetree and its bindings, dtsh)Thanks.
--
chris