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

tools/mpremote: Add delay and rtc commands. Extend documentation. #11659

Merged
merged 5 commits into from
Jun 2, 2023

Conversation

jimmo
Copy link
Member

@jimmo jimmo commented May 30, 2023

Minor mpremote updates:

  • Add delay command allowing a pause between commands.
  • Replace the setrtc "shortcut" command with a new rtc [--set] command which allows getting/setting the RTC (to the current local time).
  • Add a backwards compatible shortcut for setrtc.
  • Fix argument handling for shortcuts (to allow terminator).
  • Handle cp with no destination.

Extensive documentation update for mpremote:

  • Add a extra detail to each of the commands.
  • Add more about handling options and arguments.
  • Include shortcut commands that behave like real commands to the
    command list (e.g. bootloader, rtc).
  • Add extra information and reword to address common misconceptions, in
    particular how commands chain together.
  • Add additional examples showing some more interesting combinations.
  • Add descriptions to each of the examples.
  • Add pipx installation instructions.
  • Describe how user-configuration works.

This work was funded in combination through GitHub Sponsors and Google Season of Docs.

@github-actions
Copy link

Code size report:

   bare-arm:    +0 +0.000% 
minimal x86:    +0 +0.000% 
   unix x64:    +0 +0.000% standard
      stm32:    +0 +0.000% PYBV10
        rp2:    +0 +0.000% PICO

@codecov
Copy link

codecov bot commented May 30, 2023

Codecov Report

Merging #11659 (f59f4d5) into master (e4886dd) will decrease coverage by 0.02%.
The diff coverage is n/a.

❗ Current head f59f4d5 differs from pull request most recent head bd5d016. Consider uploading reports for the commit bd5d016 to get more accurate results

@@            Coverage Diff             @@
##           master   #11659      +/-   ##
==========================================
- Coverage   98.40%   98.39%   -0.02%     
==========================================
  Files         156      156              
  Lines       20609    20614       +5     
==========================================
+ Hits        20281    20283       +2     
- Misses        328      331       +3     

see 7 files with indirect coverage changes

@Wind-stormger
Copy link
Contributor

delay seems to allow most ESP32 boards using USB-CDC to reconnect and perform subsequent actions after a hardware reset.

Copy link
Sponsor Contributor

@mattytrentini mattytrentini left a comment

Choose a reason for hiding this comment

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

A great improvement to the mpremote docs!

docs/reference/mpremote.rst Outdated Show resolved Hide resolved

.. code-block:: bash

$ mpremote fs <command>
$ mpremote fs <sub-command>
Copy link
Sponsor Contributor

Choose a reason for hiding this comment

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

Many people I've shown mpremote to are confused by fs. "Why bother using it when all the sub-commands are commands anyway"? Explaining that they are implemented as shortcuts doesn't help...I'm not sure what the solution is here, just sharing the feedback!

Copy link
Member Author

Choose a reason for hiding this comment

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

I am confused by this too :p

I've added a Note: to this section, to at least acknowledge that this is a thing and not some extra magic.


.. code-block:: bash

$ pipx run mpremote
Copy link
Sponsor Contributor

Choose a reason for hiding this comment

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

I'd prefer to see pipx install mpremote as the example - make it easy for beginners to just copy 'n' paste. run is handy but less useful.

I'd also be sorely tempted to place pipx ahead of pip; it's just a better way to install the tool. But I get that pip is more...standard.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've updated it, see what you think.

$ mpremote run <file>
$ mpremote run <file.py>

This will copy the file to RAM on the device and execute it directly.
Copy link
Sponsor Contributor

Choose a reason for hiding this comment

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

Perhaps with less ambiguity:

Suggested change
This will copy the file to RAM on the device and execute it directly.
Transfer <file.py> to the device where it will be compiled and executed.

(Mainly because it sounded like it was copied to a RAM disk.)

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure about the suggested wording because to me "transfer to the device" implies copying it to the filesystem. I've updated the wording to not say "copy". (And sentence about what this command is useful for).

docs/reference/mpremote.rst Outdated Show resolved Hide resolved
docs/reference/mpremote.rst Outdated Show resolved Hide resolved
- mount the local directory on the remote device:
.. _mpremote_command_mount:

- **mount** -- mount the local directory on the remote device:

.. code-block:: bash

$ mpremote mount [options] <local-dir>
Copy link
Sponsor Contributor

Choose a reason for hiding this comment

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

This is a good improvement - for a very difficult topic to explain!

I wonder if this should have a short animated gif? To show a simple sequence like:

  • ls device - empty
  • ls PC - foo.py
  • mount, ls device - foo.py

Or maybe we just need to record a video, as we've discussed.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep. +1 to video.

Shortcuts can be defined using the macro system. Built-in shortcuts are::
Shortcuts can be defined using the macro system. Built-in shortcuts are:

- ``devs``: Alias for ``connect list``
Copy link
Sponsor Contributor

Choose a reason for hiding this comment

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

FWIW, I'm often asked how devs filters the serial device list to detect MicroPython devices.

Copy link
Member Author

Choose a reason for hiding this comment

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

devs (i.e.connect list) doesn't filter, but auto does, so I have explained that.

docs/reference/mpremote.rst Outdated Show resolved Hide resolved
docs/reference/mpremote.rst Outdated Show resolved Hide resolved
@jimmo
Copy link
Member Author

jimmo commented May 31, 2023

Thanks @mattytrentini -- updated the branch.

@dpgeorge dpgeorge added the tools label Jun 2, 2023
@@ -212,6 +222,10 @@ def argparse_none(description):
do_connect,
argparse_connect,
),
"delay": (
Copy link
Member

Choose a reason for hiding this comment

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

maybe this should be delay_ms to be clear on the units (and match sleep_ms)?

Copy link
Member Author

Choose a reason for hiding this comment

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

I went with millis to match the existing args to reset/bootloader, but the new sleep command kind of makes those arguments unnecessary, so removed them instead. sleep now uses seconds (to match unix sleep(1)).

"exec",
"import machine; machine.RTC().datetime((2020, 1, 1, 0, 10, 0, 0, 0))",
],
"setrtc": "rtc --set", # Backwards compatible alias for old command.
Copy link
Member

Choose a reason for hiding this comment

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

Since this never really worked as intended (it set a fixed time...) I think it's safe to just delete it, and the user can add their own shortcut if needed (and this new setrtc alias is not doing the same thing as the old command).

Copy link
Member Author

Choose a reason for hiding this comment

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

OK!

@jimmo jimmo force-pushed the mpremote-docs-update branch 2 times, most recently from a193e50 to edf33ea Compare June 2, 2023 06:06
jimmo added 5 commits June 2, 2023 16:11
This allows the sequence to be paused (e.g. wait for device, etc).

Also removes the t_ms arg in reset/bootloader, because these arguments
don't really need to be changed, and keeping them would mean inconsistent
units used for time delays.

This work was funded through GitHub Sponsors.

Signed-off-by: Jim Mussared <jim.mussared@gmail.com>
For example, the `reset` shortcut previously allowed an optional delay, but
the argument handling cannot handle `reset next-command` as `next-command`
will be interpreted as the delay argument.  The fix in this commit allows
`reset + next-command`.

This work was funded through GitHub Sponsors.

Signed-off-by: Jim Mussared <jim.mussared@gmail.com>
Replaces the existing functionality provided by the `setrtc` alias to use
the current time, rather than a hard-coded date/time.

Use `rtc` to query the current time.  Use `rtc --set` to set it.

This work was funded through GitHub Sponsors.

Signed-off-by: Jim Mussared <jim.mussared@gmail.com>
This work was funded through GitHub Sponsors.

Signed-off-by: Jim Mussared <jim.mussared@gmail.com>
Changes in this commit:
- Add a extra detail to each of the commands.
- Add more about handling options and arguments.
- Include shortcut commands that behave like real commands to the command
  list (e.g. bootloader, rtc).
- Add extra information and reword to address common misconceptions, in
  particular how commands chain together.
- Add additional examples showing some more interesting combinations.
- Add descriptions to each of the examples.
- Add pipx installation instructions.
- Describe how user-configuration works.

This work was sponsored by Google Season of Docs.

Signed-off-by: Jim Mussared <jim.mussared@gmail.com>
@dpgeorge dpgeorge merged commit bd5d016 into micropython:master Jun 2, 2023
43 checks passed
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

4 participants