Harmless system DT prep work #51835
Merged
Harmless system DT prep work #51835
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Initialize all the public API interface related attributes within the constructor instead of scattering them throughout the implementation of the class, and make sure they all have type annotations. Move all the parsing code away from the init routines and public API down to the main parsing block. This is for readability and paves the way for later changes that affect the way initialization happens. Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Reorder attribute initialization to match the order that attributes appear in the class level docstring. This is for readability. Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Make attribute initialization order match the order that attributes appear within the class docstring. Move the 'type' property definition up by the constructor to make it more obvious that this 'attribute' is a (Python) property. This is for readability. Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Holy overloaded technical terms, Batman. Here, 'property' and 'type' each mean two different things, which we can distinguish like this: - Property (capital P): dtlib.Property class, represents a property in a devicetree node - @Property: a Python property - type(): an "@Property" in the Property class, that returns a dtlib.Type value - Type (capital T): dtlib.Type class, represents the devicetree type of a Property value (dtlib.Type.BYTES, etc.) The type() @Property in the Property class currently has an 'int' as its Python return type annotation. It really returns a dtlib.Type, which is an int (since it's an IntEnum), but that's not the same thing as an int. Change this to Type to be clear that not just any int can be returned by this @Property. Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Refactor the file parsing methods for readability by moving the _parse_header() and _parse_memreserves() calls from _parse_dt() to _parse_file(). The header and memreserves are not part of the 'tree' part of the devicetree; now that we have a dedicated _parse_file() helper, it makes more sense to me to have them there. Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
The value of the 'labels' attribute is a list, not a set. Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Regular dicts are insertion-ordered since CPython 3.6 and Python 3.7.
Zephyr now requires Python 3.8, so it should be OK to replace
OrderedDict with regular dict now. This results in less typing and
more readable object representations.
A nitpicker could argue that this is a functional change, since if a
user is doing 'assert isinstance(node.props, OrderedDict)', that will
fail now, but:
1. nobody is doing something like that in the zephyr tree
2. that would be a silly thing to do
3. we don't currently make any API stability guarantees
for this module right now anyway
so it should be fine.
Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
mbolivar-nordic
force-pushed
the
sysdt-prep-work
branch
from
7b62d26 to
33f66c4
Compare
The standard library copy module allows you to implement shallow and deep copies of objects. See its documentation for more details on these terms. Implementing copy.deepcopy() support for DT objects will allow us to "clone" devicetree objects in other classes. This in turn will enable new features, such as native system devicetree support, within the python-devicetree. It is also a pure feature extension which can't harm anything and is therefore safe to merge now, even if system devicetree is never adopted in Zephyr. Note that we are making use of the move from OrderedDict to regular dict to make this implementation more convenient. See https://github.com/devicetree-org/lopper/ for more information on system devicetree. We want to add system devicetree support to dtlib because it seems to be a useful way to model modern, heterogeneous SoCs than traditional devicetree, which can really only model a single CPU "cluster" within such an SoC. In order to create 'regular' devicetrees from a system devicetree, we will want a programming interface that does the following: 1. parse the system devicetree 2. receive the desired transformations on it 3. perform the desired transformations to make a 'regular' devicetree Step 3 can be done as a destructive modification on an object-oriented representation of a system devicetree, and that's the approach we will take in python-devicetree. It will therefore be convenient to have an efficient deepcopy implementation to be able to preserve the original system devicetree and the derived regular devicetree in memory in the same python process. Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
At some point in the past, we had to suppress a couple of false positive pylint warnings to pass CI. But now the linter seems to have figured out its original mistake and is complaining about a useless supression. Sigh. Play along. Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
mbolivar-nordic
force-pushed
the
sysdt-prep-work
branch
from
33f66c4 to
b30f4d7
Compare
|
Addressed linter complaints. |
gmarull
approved these changes
Nov 2, 2022
galak
approved these changes
Nov 7, 2022
|
Is there anywhere we can check about the python version? Not critical, but would be nice to say you need 3.6 or greater somewhere for dtlib. |
We should This works if added to one of our We have the bad habit of hacking Python path in most scripts, this would remove such need plus offer checks like Python version, dependencies, etc. |
Right, as far as the python devicetree package is concerned, what @gmarull said is true: we already have this declared in setup.py. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A collection of fixes, enhancements, and clean-ups to our python-devicetree package that I made while working on a prototype for #51830.
I believe this is safe to merge now.