Skip to content
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: support deepcopy and add type annotations #56849

Conversation

mbolivar-nordic
Copy link
Contributor

@mbolivar-nordic mbolivar-nordic commented Apr 14, 2023

This is a standalone PR that will be needed to add system devicetree support.

After prototyping in #52272, it became clear that it makes sense to implement the sysdtlib.SysDT class we will use to implement system DT support as a subclass of edtlib.EDT. This is a change from making it a subclass of dtlib.DT as originally proposed.

Using edtlib instead means that we can have type-safe access to some properties in sysdtlib by writing bindings for system DT specific compatibles. It also helps when implementing requirements like accessing the proper number of cells in reg properties, which edtlib knows, but dtlib doesn't. (We need address/size cell counts in order to be able to decode the address-map properties in CPU cluster nodes.)

This PR does two things that we need to be able to safely do this subclassing:

  1. Adds EDT.__deepcopy__() support, so that I can make a copy of the system devicetree, mutate it into a regular devicetree, and emit the mutated copy
  2. Adds type annotations for all the public classes in edtlib, so that sysdtlib can be implemented in a type-safe way

There are a couple of cleanup commits at the beginning that I found along the way.

The reference is to an incorrect method.

Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
This is still the preferred style in a zephyr DTS.

Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Move all the initial settings of instance attributes to the
constructor, so we can keep track of them all more easily.

No functional changes expected.

Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Just like we did for dtlib in 15e3e31
("dtlib: implement copy.deepcopy() for DT"), except this time it's for
EDT. This also can do no harm and will be useful for implementing
system devicetree support.

No functional changes expected under the assumption that no users are
relying on us having stashed the exact bindings_dirs list passed to
the constructor. This patch switches to making a defensive copy, which
is safer and makes implementing this a little cleaner.

Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
This is just moving the class definition higher in the file. I am
reordering the classes to make it possible to type annotate the module
in a more readable way.

Git might make the diff look bigger than it really is.
To verify this is just moving code, use 'git diff --minimal'.

Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
This is just moving the class definition higher in the file
to make it easier to type annotate the module.

Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
This is just moving the class definition higher in the file
to make it easier to type annotate the module.

Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
This is just moving the class definition higher in the file
to make it easier to type annotate the module.

Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
This is just moving the class definition higher in the file
to make it easier to type annotate the module.

Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
This is just moving the class definition higher in the file
to make it easier to type annotate the module.

Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
This is just moving the class definition higher in the file
to make it easier to type annotate the module.

Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
This is just moving the class definition higher in the file. I am
reordering the classes to make it possible to type annotate the module
in a more readable way.

Git might make the diff look bigger than it really is.
To verify this is just moving code, use 'git diff --minimal'.

Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Miscellaneous fixes discovered by inspection while type annotating the
module.

Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Incremental progress towards type annotating the whole module.
Annotate helper procedures used by the class as well.

Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Incremental progress towards type annotating the module.

Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
@mbolivar-nordic mbolivar-nordic force-pushed the edtlib-deepcopy-and-types branch from ec5b6ba to 1505174 Compare April 14, 2023 10:50
Converting this to a dataclass will make it easier to type annotate.
Adding type annotations is incremental progress towards type checking
the entire module.

Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Converting this to a dataclass will make it easier to type annotate.
Adding type annotations is incremental progress towards type checking
the entire module.

Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Fix grammar issues and typos.

Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Converting this to a dataclass will make it easier to type annotate.
Adding type annotations is incremental progress towards type checking
the entire module.

Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Converting this to a dataclass will make it easier to type annotate.
Adding type annotations is incremental progress towards type checking
the entire module.

Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Converting this to a dataclass will make it easier to type annotate.
Adding type annotations is incremental progress towards type checking
the entire module.

Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
This requires adding a private constructor so that mypy
can tell what all the final instance state is going to be.

Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Incremental progress towards type annotating the entire module.

Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
This concludes the type annotations for the public API for the module,
along with the relevant internal state. It's not worth type annotating
the internal backwards compatibility shim for !include.

Signed-off-by: Martí Bolívar <marti.bolivar@nordicsemi.no>
Copy link
Collaborator

@nordicjm nordicjm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks okay

@mbolivar-nordic mbolivar-nordic merged commit 5847890 into zephyrproject-rtos:main Apr 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants