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

RFC — DtSh, shell-like interface with Devicetree #59863

Open
wants to merge 30 commits into
base: main
Choose a base branch
from

Conversation

dottspina
Copy link
Contributor

@dottspina dottspina commented Jun 30, 2023

RFC: DTSh: DTS file viewer with a shell-like command line interface

Introduction

Problem description

Prior to Zephyr, I've only approached Devicetree while looking through LKML or The Linux Kernel documentation.

And, quoting @mbolivar-nordic (October 2020):

Is your enhancement proposal related to a problem? Please
describe.

Devicetree is widely used by Zephyr, but it lacks convenient tooling
for development.

Describe the solution you'd like

A graphical devicetree viewer and editor, like 'guiconfig' for DTS.

I can agree that, when learning Zephyr use of Devicetree, I've personally felt the lack of a quick, simple tool to:

  • see the final devicetree generated at build-time
  • find nodes based on supported bus protocols, generated IRQs, or keywords like "BME680" or "PWM"
  • access the relevant binding files
  • generate simple figures to illustrate my note-taking

References:

Proposed change

The proposed change is a new West command that opens a Devicetree in a shell-like command line interface:

  • browse a devicetree through a hierarchical file system metaphor
  • search for devices, bindings, buses or interrupts with flexible criteria
  • filter, sort and format commands output
  • generate simple documentation artifacts (text, HTML, SVG) by redirecting the output
    of commands to files
  • rich Textual User Interface, command line auto-completion, command history, user themes
$ west build
$ west dtsh
dtsh (0.0.99): Shell-like interface with Devicetree
How to exit: q, or quit, or exit, or press Ctrl-D

/
> cd &flash_controller

/soc/flash-controller@4001e000
> find -E --also-known-as (image|storage).* --format NKd -T
                             Also Known As               Description
                             ───────────────────────────────────────────────────────────────────────────────────
flash-controller@4001e000    flash_controller            Nordic NVMC (Non-Volatile Memory Controller)
└── flash@0                  flash0                      Flash node
    └── partitions                                       This binding is used to describe fixed partitions…
        ├── partition@c000   image-0, slot0_partition    Each child node of the fixed-partitions node represents…
        ├── partition@82000  image-1, slot1_partition    Each child node of the fixed-partitions node represents…
        └── partition@f8000  storage, storage_partition  Each child node of the fixed-partitions node represents…

where:

  • west dtsh: when invoked without any argument, the Devicetree shell will open the DTS file build/zephyr/zephyr.dts, and retrieve the bindings Zephyr has used at build-time from the CMake cache file build/CMakeCache.txt
  • cd &flash_controller changes the current working directory from the devicetree's root to the node at /soc/flash-controller@4001e000, using its DTS label flash_controller
  • find is with no surprise a shell command that will search for devices, bindings, buses, interrupts, etc
  • here -E --also-known-as (image|storage).* will match nodes with a label or alias starting with image or storage; predicates like --with-reg-size >4k are also supported
  • --format NKd: set the node output format to the columns "node Name,", "Also Known As" (all labels and aliases), and "description" (D is the "Depends-on" column)
  • the descriptions, e.g. "Flash node", are actually hyperlinks that, when clicked, will open the corresponding binding files
  • -T: list found nodes in tree-like format
  • appending > doc/partitions.svg to the last command line would save the partitions tree to the file doc/partitions.svg, in SVG format

The considered use cases include:

  • to help getting started with Devicetree: hierarchically or semantically explore a devicetree, contextually access binding files or the Devicetree specification, save figures to illustrate notes
  • to have on hand a simple DTS file viewer: quickly check the enabled buses and connected devices or the RAM and Flash memory, get a first insight when debugging a Devicetree issue, document hardware configurations
  • an extensible framework for prototyping TUI tools based on Zephyr's python-devicetree library

This proposal started with a Proof of Concept project: source code and documentation for this prototype are still available as the main branch of the DTSh repository.

The new code base in this RFC is also distributed as a stand-alone package you can install from PyPI without messing with your Zephyr wokspaces: for details, take a look the dtsh-next branch which mirrors this PR in the DTSh repository. Note that this version can't provide West integration: just replace west dtsh with dtsh.

Please refer to the attached DTSh User Guide (PDF) for the complete documentation of what this RFC implements and examples of use.

Detailed RFC

This RFC includes:

  • the Devicetree Shell (DTSh) implementation in Python and its unit tests
  • a West command for opening DTS files with DTSh
  • documentation in reStructuredText format

Proposed change (Detailed)

This RFC wouldn't be review-able as a single commit: it is introduced as a series, where each commit adds a new feature or API, with its companion unit tests when they exist.

Once reviewed, we can squash or rearrange the whole thing according to the maintainers' preferences.

DTSh

DTSh is implemented in Python and is a Devicetree tool: it seems its natural location is zephyr/scripts/dts/dtsh, sibling to python-devicetree with which it's tightly coupled.

Source files are located in dtsh/src, and dtsh is the root Python package.

Unit tests are implemented with pytest, and located in dtsh/tests.
The res sub-directory contains test resource files.

Devicetree model

DTSh introduces its own model layer above edtlib.EDT.

Rationale:

  • factorize DTSh interface with the edtlib API (python-devicetree)
  • support the hierarchical file system metaphor at the model layer
  • unified API for sorting and matching nodes
  • write most unit tests at the model layer
Module Unit tests
dtsh.dts devicetree source definition test_dtsh_dts
dtsh.model devicetree model test_dtsh_model
dtsh.modelutils devicetree model helpers (e.g. sorters, criteria) test_dtsh_modelutils

GNU Readline integration

Responsibilities:

  • provide DTSh client code with a single entry-point API for GNU Readline integration
  • isolate the completion logic (find matches) and view providers (display matches)
    from the Readline hooks machinery
Module Unit tests
dtsh.rl GNU readline integration API
dtsh.autocomp base completion logic and view providers test_dtsh_autocomp

Devicetree shell

Responsibilities:

  • command line parser
  • devicetree shell sessions setup
  • helpers for implementing shell commands
Module Unit tests
dtsh.io base I/O streams for the devicetree shell test_dtsh_io
dtsh.config devicetree shell configuration test_dtsh_config
dtsh.shell devicetree shell test_dtsh_shell
dtsh.shellutils common command flags, arguments and parameters test_dtsh_shellutils
dtsh.session base devicetree shell session

rich TUI

DTSh's rich Textual User Interface is built on Texualize's rich library.

rich TUI

Module Unit tests
dtsh.rich.theme colors and styles test_dtsh_theme
dtsh.rich.tui base layouts
dtsh.rich.text factories for styled text
dtsh.rich.modelview factories for devicetree model views
dtsh.rich.svg contents format for output redirection (SVG) test_dtsh_rich_svg
dtsh.rich.io rich I/O streams for the devicetree shell
dtsh.rich.autocomp rich completion view providers
dtsh.rich.shellutils command flags and arguments for formatted output test_dtsh_rich_shellutils
dtsh.rich.session rich devicetree shell session

Built-in Commands

Bellow are the currently implemented commands.

Module Unit tests
dtsh.builtins.cd cd: change the current working branch test_dtsh_builtin_cd
dtsh.builtins.pwd pwd: print path of current working branch. test_dtsh_builtin_pwd
dtsh.builtins.ls ls: list branch contents test_dtsh_builtin_ls
dtsh.builtins.tree tree: list branch contents in tree-like format test_dtsh_builtin_tree
dtsh.builtins.find find: search branches for nodes test_dtsh_builtin_find
dtsh.builtins.alias alias: list aliased nodes test_dtsh_builtin_alias
dtsh.builtins.chosen chosen: list chosen nodes test_dtsh_builtin_chosen

West extension

West command implementation:

  • class DTShell in zephyr/scripts/west_commands/dtshell.py
  • packages dtsh and devicetree imported with os.sys.insert() statements

Added dtsh extension to West in zephyr/scripts/west-commands.yml.

Added DTSh run-time requirements to zephyr/scripts/requirements-base.txt:

Added support for West completion to west-completion.bash and west-completion.zsh.

Documentation

The proposed change:

  • adds a brief entry in zephyr-cmds.rst to document the West command
  • that links to a detailed user guide (dtsh.rst) in the same directory

Dependencies

DTSh should not affect other components or needs changes in other areas.

But that's not true the other way around: changes in the Zephyr's hardware model may require changes in DTSh (see bellow).

Concerns and Unresolved Questions

Maintenance

This is a valid concern:

  • the code base is sizable
  • though Zephyr won't depend on DTSh in any way, DTSh is tightly coupled with Zephyr's hardware model and the companion edtlib API: this might introduce some additional maintenance for it to consistently follow changes

The everyday maintenance should be low, though:

  • the Python module dtsh.model encapsulates all uses of the edtlib API: if breaking changes are introduced, DTSh will fail there
  • the included unit tests should catch the most obvious regressions early, and help narrowing issues

The above observations come from my own experience (an example of breaking change was identifying top-level bindings with (compat, bus) tuples rather than just compatible strings).

More significant changes, like "Hardware model v2" or "Better support for multi-core AMP SoCs", could however involve more work, to either fix DTSh or add support for the new model.

Fortunately:

  • such model changes don't happen every full moon night
  • Zephyr seems to operate this kind of transitions cautiously: changes are discussed in advance, and introduced incrementally with some backward compatibility

Finally, I'll gladly continue myself contributing support, features and bug fixing in the long run.

Limitations

This RFC is not a complete Devicetree shell:

  • there's no syntax to access an arbitrary property's value or description (binding): this will be the responsibility of a cat command, with a syntax like cat soc/timer$max-bit-width
  • it misses a versatile man command, with manual pages for DTSh itself, but more importantly also for bindings and boards

I've already implemented these when experimenting with the PoC project: I know what to do, and how.

But this RFC is already sizable, and will require quite some work from its reviewers: I'd prefer we first agree on the initial design, implementation and documentation, then I'll push the missing commands with shortest individual PRs. This has been discussed and so far seems to be a consensus.

GNU Realine on Windows

Or the lack of.

For auto-completion, command history and key bindings, DTSh relies on the GNU Readline variant of the standard Python readline API.

Basically, there's no sane and straight forward way to build and distribute the GNU Readline library on non POSIX systems: on Windows, Python simply gave up and does no longer include the readline module with its standard library (see e.g. There's no readline module on Windows Python (cmd.Cmd) for some historical context).

The stand-alone module pyrealine3 cited in the above reference has been investigated as a work-around, but its API is incomplete, and its implementation too much coupled with cmd.Cmd, which we don't use. The project does not seem actively maintained.

To get a cross-platform user experience, I've also considered the pure Python Prompt Toolkit:

  • what I've been able to achieve was clearly disappointing compared to binding native GNU Readline hooks to rich callbacks
  • using both the Prompt Toolkit and Textualize's rich library was painful, and introduced other issues

Although DTSh may eventually work around this limitation, this is clearly not a priority at the moment.

As a consequence, the GNU Readline integration will very likely be disabled on Windows, resulting in a degraded user experience.

However, I am quite optimistic that this will not be considered a merge blocker: Zephyr seems to admit this kind of little (though very unfortunate here) difference between POSIX and non POSIX systems, e.g. west itself also supports auto-completion only with the Bash and Zsh shells.

User Guide Integration

The DTSh User Guide (dtsh.rst) is quite long, and contains sections, subsections, sub-subsections, etc.

Its integration with Zephyr's documentation is almost fine in HTML, and I'm not sure how to do better without messing with the :toctree:, which I prefer to avoid.

Its integration when generating PDF is more debatable (look at the attached PDF, concerns should be obvious):

  • since the User Guide's content in the PDF document already starts at the "Second section heading level", proper styles quickly run out to highlight its own headings
  • for the same reason, the TOC pane (typically shown at left in PDF viewers) will flatten the User Guide to a single heading level

The final PDF document is still legible, but I admit it's not optimal: I will gladly try any ideas for improvement.

Correctness

DTSh use cases include educational tools for newcomers to Zephyr Devicetree: if not correct, erroneous results (commands output) will do more harm than good.

Although correctness is the focus of unit tests, and code coverage for the core modules should be almost complete, these tests involve only a small subset of the possible configurations (MCU, SoC, boards, shields, peripherals).

I think we could define a minimum level of correctness as:

  • DTSh should not fault when opening any valid Devicetree source (DTS and bindings), targeting any hardware configuration
  • with these, all example commands found in the User Guide should consistently execute: important information specific to some configuration might be missing because unanticipated, but commands should not produce incorrect or misleading output

Security

The proposed change is not that hazardous:

  • dtsh does not evaluate (eval()) any part of the user input
  • dtsh does not pipe commands to the system interpreter (e.g. os.system() or os.popen()), nor create any kind of child process (e.g. subprocess.run())
  • Devicetree source and binding files, CMake cache files, etc, are all opened read-only
  • the writable user specific files (command history, custom configuration and theme files) are written to (and read from) a per-user application data directory located under the appropriate platform-dependent root (e.g. $XDG_CONFIG_HOME or %LOCALAPPDATA%)
  • by default dtsh won't override any existing file (including when redirecting commands output)
  • the West command itself is rather simple

Personal biases

Why a command line application ? Well, when you've done west build, you're already at the command line, and I think continuing from there with west dtsh, getting a different prompt but the same user interface paradigms and even key bindings (the base ones used by Zsh, GNU Bash, Emacs, GDB), is more ditrsaction-free than opening a GUI.

Beyond this personal biases, things like PyGObject or Qt for Python have nonetheless been considered, but:

  • these would be huge requirements to add to Zephyr, and to the users' workspaces
  • I wanted something simple and fast, easy to extend and to prototype with
  • at the end of the day, I'm not sure a full GUI will be more user-friendly or informative than what is possible with TUI/CLI

Alternatives

There doesn't seem to be many DTS file viewers.

Nordic Semiconductor's distributes the nRF DeviceTree extension for VS Code, which looks interesting but does not address the initial problem (as @hongshui3000 also pointed out):

  • it's not Open Source, and seems to assume you're using the nRF Connect SDK, not Zephyr
  • VS Code with the nRF Connect Extension Pack is not really a simple tool everyone is supposed to have on hand (or to work with)

@dottspina dottspina marked this pull request as draft June 30, 2023 06:07
@dottspina dottspina force-pushed the rfc-dtsh branch 2 times, most recently from 7057216 to 66b9925 Compare June 30, 2023 07:27
@dottspina dottspina changed the title Rfc dtsh RFC — DtSh, shell-like interface with Devicetree Jun 30, 2023
@dottspina dottspina force-pushed the rfc-dtsh branch 6 times, most recently from 7f86207 to b170331 Compare July 3, 2023 23:27
@hongshui3000
Copy link
Contributor

I think VS Code extension: nRF DeviceTree is a good graphical tool. But it's not open source. Not particularly friendly to non-nRF devices

@mbolivar-ampere
Copy link
Contributor

Thanks for this. I think it's a great idea and I especially appreciate the attention to detail you put into the RFC as well as volunteering maintenance. Consider me onboard with this idea. I'll give it some testing and review.

@mbolivar-ampere
Copy link
Contributor

Some unboxing comments:

Python provided by Homebrew or Fink: should also work OOTB

I can't get tab completion working as I expect using the homebrew python 3.11. From the prompt:

/
❭ 

I press TAB and expect commands to be completed. But instead I see a tab character is inserted. Is that expected? Here is my python info (using a virtual environment):

(devel) mbolivar@PDX-FY74HK3HFV zephyr % python -V
Python 3.11.4
(devel) mbolivar@PDX-FY74HK3HFV zephyr % echo $(readlink $(which python))
/opt/homebrew/opt/python@3.11/bin/python3.11

I can import readline from the python REPL with no error. I did a pip install -r zp/zephyr/scripts/dts/dtsh/requirements.txt before running west dtsh.

The command parser also doesn't seem to be stripping leading whitespace:

/
❭     q
dtsh: command not found: q

/
❭ q
bye.

this isn't like what you'd expect from a shell in my opinion -- I would expect both of those to quit.

Command options to configure formatted outputs.

Base command with boilerplate code to support
formatted output.

Unit tests and examples: tests/test_dtsh_rich_shellutils.py

Signed-off-by: Christophe Dufaza <chris@openmarl.org>
Unit tests and examples: tests/test_dtsh_builtin_pwd.py

Signed-off-by: Christophe Dufaza <chris@openmarl.org>
Unit tests and examples: tests/test_dtsh_builtin_cd.py

Signed-off-by: Christophe Dufaza <chris@openmarl.org>
Unit tests and examples: tests/test_dtsh_builtin_ls.py

Signed-off-by: Christophe Dufaza <chris@openmarl.org>
Unit tests and examples: tests/test_dtsh_builtin_tree.py

Signed-off-by: Christophe Dufaza <chris@openmarl.org>
Search for nodes with pre-defined criteria.

Unit tests and examples: tests/test_dtsh_builtin_find.py

Signed-off-by: Christophe Dufaza <chris@openmarl.org>
Print information about aliased nodes.

Unit tests and examples: tests/test_dtsh_builtin_alias.py

Signed-off-by: Christophe Dufaza <chris@openmarl.org>
Print information about chosen nodes.

Unit tests and examples: tests/test_dtsh_builtin_chosen.py

Signed-off-by: Christophe Dufaza <chris@openmarl.org>
Base interactive devicetree shell session.

A session binds a devicetree shell and a VT
to run an interactive loop.

Signed-off-by: Christophe Dufaza <chris@openmarl.org>
Rich interactive devicetree shell session.

Extend the base session with a few rich TUI elements
and support for SVG and HTML command output redirection formats.

Signed-off-by: Christophe Dufaza <chris@openmarl.org>
Run the devicetree shell without West.

Signed-off-by: Christophe Dufaza <chris@openmarl.org>
@dottspina
Copy link
Contributor Author

Assuming the reviewers assigned to this PR were all busy working on the "Introduction of board and SoC scheme v2" and "Better support for multi-core AMP SoCs" (which look interesting), I haven't pushed nor commented in two month, and this PR has eventually been automatically closed by a bot.
And, although the message ends with "Note, that you can always re-open a closed pull request at any time.", I couldn't found how when I tried to update this PR.

IIUC (e.g. issue #59177 and Allow to reopen pull requests after a force push):

  • to have a chance to reopen this PR, it must contains the exact same commit that was there when the PR was closed: I found 8a0a160 in my git reflog and was able to revert the PR's origin to this previous state
  • I can't reopen myself a PR that was closed by the bot, I have to either open a new PR, which doesn't seem like a good practice to me, or request support from someone with higher privileges: @mbolivar-ampere , could you please try to reopen this ?

I should then be able to rebase it with the actual contents that's ready there.

Changes since November include (history has been squashed numerous times, this is what comes to mind):

  • by default, ls, tree and find should behave more closely to their Unix homonyms: this may sound a bit low priority, but actually helps understanding the output, e.g. why the same node will appear with different paths like ./soc/timer@e000e010, soc/timer@e000e010 or /soc/timer@e000e010, depending on the command and the arguments it was executed with (the Unix homonyms can serve as reference)
  • rewritten commands output redirection: the design was not satisfactory, and appending commands output to an existing HTML or SVG file was entirely broken: you can't just append bytes to structured contents (this now works good enough)
  • a couple of other bug fixes and small improvements in various areas (TUI, configuration, West command)
  • some final refactoring and other code cleanup
  • updated user guide and RFC

I thinks this PR, once properly rebased, will be ready for review.

It was a personal choice not to communicate (e.g. Zephyr mailing lists) too early, I think now is the time and I will do it once this is reopened to also get some user oriented feedback.

Thanks.

@pdgendt pdgendt reopened this Jan 24, 2024
@pdgendt
Copy link
Collaborator

pdgendt commented Jan 24, 2024

@dottspina I've reopened the PR for you.

West command implementation:
- class DTShell in zephyr/scripts/west_commands/dtshell.py
- packages dtsh and devicetree imported with os.sys.insert()
  statements

Add dtsh extension to West in zephyr/scripts/west-commands.yml.

Add dtsh requirements to "used by west_commands"
in zephyr/scripts/requirements-base.txt:
- rich (https://github.com/Textualize/rich)
- Pygments (https://github.com/pygments/pygments)
- gnureadline (https://pypi.org/project/gnureadline/), macOS only

Add support for West completion in:
- zephyr/scripts/west_commands/completion/west-completion.bash
- zephyr/scripts/west_commands/completion/west-completion.zsh

Signed-off-by: Christophe Dufaza <chris@openmarl.org>
Add to Zephyr documentation:
- West command usage
- DTSh user guide

Signed-off-by: Christophe Dufaza <chris@openmarl.org>
@dottspina
Copy link
Contributor Author

dottspina commented Jan 24, 2024

@dottspina I've reopened the PR for you.

Thank you @pdgendt.
The PR seems to have properly recovered now, and I've been able to push the actual rebase.

[Edit: the "Stale" label has also been removed]

@dottspina dottspina marked this pull request as ready for review January 24, 2024 08:12
@github-actions github-actions bot removed the Stale label Jan 25, 2024
@JPHutchins
Copy link

This looks like an absolutely amazing tool with a lot of power to save everyone time, nice work!

I have a strong preference for dependencies like this to live as PyPI packages so that they can:

  1. be maintained in isolation of the Zephyr repository
  2. allow Zephyr to easily pin versions
  3. allow users to change the version of a dependency without changing the version of Zephyr
  4. be used by projects other than Zephyr (more easily)

How would you feel about publishing to PyPI and only committing the West integration to the Zephyr repo?

dottspina added a commit to dottspina/dtsh that referenced this pull request Feb 8, 2024
Backport RFC-DTSh code base for dtsh-0.2rc1.

See zephyrproject-rtos/zephyr#59863
dottspina added a commit to dottspina/dtsh that referenced this pull request Feb 9, 2024
dottspina added a commit to dottspina/dtsh that referenced this pull request Feb 9, 2024
@dottspina
Copy link
Contributor Author

Thanks for your feedback, @JPHutchins.

I have a strong preference for dependencies like this to live as PyPI packages so that they can:

I agree that maintenance should not be overlooked: this concern is part of the RFC, and I indeed expect it to be discussed.

1. be maintained in isolation of the Zephyr repository

Dependencies first go the other way: although this RFC/PR does not affect Zephyr aside from the new West command, DTSh is tightly coupled to the Zephyr's python-devicetree library, and to the hardware model it's an API for.

Therefore, when distributed independently, DTSh releases have to pin the python-devicetree versions they're developed and tested with:

  • either as a Python requirement that will install the appropriate version along with DTSh: making this library available independently is not a priority of the Zephyr project, and the PyPI package is no longer maintained, which rules out this option
  • or by re-distributing snapshots of the appropriate versions of the library: this is not ideal, but doable, see e.g. ci: bundle devicetree Python package

One way or another, Zephyr users will end up with two instances of the python-devicetree library:

  • the one that comes with Zephyr, on which it will rely at build-time
  • the one installed by/with DTSh

Which raises the question of what should happen when running west dtsh:

  • should DTSh unconditionally use the pinned version of python-devicetree it has installed (it depends on): since west build will still use the library that comes with Zephyr, chances are that west dtsh will use a different version, which might prove confusing or misleading
  • should DTSh first search for some "current working Zephyr" (think ZEPHYR_BASE), and prefer the python-devicetree library found there instead: west build and west dtsh will then consistently use the same version, but now chances are that you won't run DTSh using the API it's been developed and tested with (it depends on)
2. allow Zephyr to easily pin versions

If Zephyr in turn pins DTSh, we add mutual dependencies to the picture.

Although these might be handled through a coordinated release (and testing !) process, I'm afraid this will turn out painful for maintainers and sub-optimal for users, especially when releasing and distributing bug fixes.

3. allow users to change the version of a dependency without changing the version of Zephyr

To me, users needing to change the version of DTSh they are using with a given version of Zephyr would show that the above coordinated release process is indeed a failure.

4. be used by projects other than Zephyr (more easily)

DTSh still makes little sense outside of Zephyr, as does the python-devicetree library: both might eventually be helpful with e.g. the Linux kernel's Devicetree, but AFAICT we're still far from that.

This doesn't mean DTSh can't be installed independently of Zephyr, and tried to experiment with arbitrary DTS and binding files, see bellow.

How would you feel about publishing to PyPI and only committing the West integration to the Zephyr repo?

Aside from the personal concerns I've already mentioned, I don't think making the DTSh shell an external dependency installed as a Zephyr requirement, while the West command would be maintained upstream, can be acceptable to Zephyr maintainers:

  • DTSh being developed and released independently won't save them from reviewing and testing the DTSh versions distributed (pinned) with Zephyr to ensure appropriate levels of quality and security: it will just make the process more painful compared to managing pull requests and being able to add unit and integration tests to the project's CI
  • installing DTSh from PyPI implies guarantees that a code review alone can't provide: even if you trust the project maintainer as a person, you also have to trust the publishing process, e.g. Do the repository tags match the release numbers on PyPI ? or How are managed the SSH keys used for authenticating uploads ?

To answer your question, my feeling is thus that if DTSh were to be maintained, as much as possible, in isolation of the Zephyr repository, we would better off abandoning the west dtsh command entirely.

Publishing on PyPI has issues, but could work: the DTSh project's default branch now mirrors and packages the code base of this RFC, that you can install and run independently of West or even Zephyr (more details here). Note that this package can't provide the West integration: just run dtsh instead of west dtsh.

Why I think having this upstream could nonetheless benefit to Zephyr users:

  • on hand and beginner-friendly: no additional installation or configuration step, just west dtsh
  • west build and west dtsh will consistently depend on the same Zephyr hardware model and API, no need to take care of some compatibility matrix
  • source code, tests, CI, documentation and issues all in one place
  • the Zephyr toolchain misses a simple mean to see the final DTS file generated at build-time like a device tree
  • IMHO, the ability to redirect (export) commands output to HTML, SVG or simple text files is also a rationale for an OOTB availability (notes-taking, educational materials, document hardware configuration)

Thanks.

dottspina added a commit to dottspina/dtsh that referenced this pull request Feb 22, 2024
Mirror and package RFC-DTSh code base,
minus West integration.

To be released as DTSh 0.2-rc1.

See zephyrproject-rtos/zephyr#59863
@dottspina
Copy link
Contributor Author

I understand really well that those who might review this RFC have other legit priorities, and I don't want to rush anyone. But I'm a bit bothered.

At some point, to not actually freeze all DTSh development until this is approved or rejected, I somewhat rebased a new project branch onto the source code in this PR minus the West command (DTSh v0.2.0-rc1), and made it the default after a few additional patches to better take into account both installation types, bundled with Zephyr or downloaded from PyPI (DTSh v0.2.0).

DTSh has since gained a batch mode, two new commands, beside bug fixes and improvements in various areas. The commit history could permit to cherry-pick just a few things, or I could rebase this PR onto the latest DTSh release (v0.2.2) or any tag in between. And that's what bothers me: I don't know what to do ?

What is certain is that reviewing this RFC as it is would make no sense: we're now far from the idea of merging this, then adding short incremental PRs (something like when the rabbit can't ever catch up with the turtle, you know).

I still personally think that developing and maintaining DTSh upstream as a Zephyr extension to West makes sense:

  • on hand and beginner-friendly: no additional installation or configuration step, just west dtsh
  • Zephyr (e.g. west build) and DTSh (west dtsh) will consistently depend on the same Zephyr hardware model and its Python API; this is otherwise an issue when a Zephyr user installs DTSh from PyPI, which has already been discussed here, but you can also take a look at ci: possible multiple different versions of python-devicetree if that doesn't sound obvious
  • the Zephyr toolchain is missing a simple default tool to open the final DTS files generated at build time, and IMHO DTSh might make a good candidate (please refer to its documentation to get an idea)
  • the ability to redirect (export) commands output to files is a basic but straight forward helper for notes-taking, to illustrate educational materials or document hardware configurations; and now all this can be automated (batch mode)

So I would lean toward rebasing this PR onto at least DTSh v0.2.1, to get the batch mode and the cat command: it doesn't add that much source code, main concerns could be changes to the possible interactions with the user's file system (it's now permitted to overwrite existing files by default when appending with >>, and DTSh can read commands from files in batch mode).

But I don't want to decide alone, go the wrong approach, and send to much time myself to /dev/null.

Any ideas or opinions, e.g. @mbolivar-nordic , @kartben , @galak ?

I'm not getting impatient, I just don't know what I should do now ;-)

Thanks.

References:

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.

None yet

7 participants