micropython / micropython

MicroPython - a lean and efficient Python implementation for microcontrollers and constrained systems
https://micropython.org
Other
19.51k stars 7.81k forks source link

mpremote: UI improvements #7602

Open cutl opened 3 years ago

cutl commented 3 years ago

I think the inconsistencies in the mpremote command set make it error prone. Commands 'mount' and 'run' default to the local FS, whereas the 'fs' commands default to the remote FS, except for 'fs cp' which uses ':' to denote remote. This is confusing:

$ mpremote cp :app . ; edit app     # make changes to app
$ mpremote cp app :trial            # upload the changed version
$ mpremote exec "import trial"      # the changes disappoint
$ mpremote rm app :trial            # discard the changed versions: *actually deletes the original*

Using ':' consistently would fix this and would also be welcome if interactive sessions are added to mpremote or if the other fs commands are brought into line with cp / scp.

Command concatenation supports an on-device command session within each 'mpremote' command. But device selection isn't performed by the device, and as part of the session environment it would be better as an option like "--dev device_name". Similarly I'd suggest 'mpremote --list' for candidate devices rather than 'connect list', and 'disconnect' seems redundant.

Commands separators are needed to resolve the ambiguity caused by 'fs' commands taking a variable number of arguments:

$ mpremote mkdir pybdir cp f :pybdir
mkdir :pybdir
mkdir :cp
mkdir :f
mkdir :pybdir
OSError: [Errno 17] EEXIST

'mount' is nice but would be clearer as 'chroot', and 'cp' clearer as 'tf' standing for transfer (or better still fix 'cp :remote1 :remote2'). These changes would improve ease of use even if users can live with some of the oddities for as long as there are only a few commands.

dpgeorge commented 2 years ago

Commands separators are needed to resolve the ambiguity caused by 'fs' commands taking a variable number of arguments:

This is now fixed by a311e9e3d419cf447c8a4cf2beed4a341e8387e3, which added + as a command terminator/separator.

CroftAr commented 2 years ago

The last time I saw '+' used as a terminator was on a telegram in a black and white movie :-) Seriously though, thanks for the fix.

The mpremote documentation seems unclear, see examples below (and issue 8272). Rshell has a clear concept of layered sessions for REPL commands and rshell commands, making the command language consistent and easy to use. In contrast mpremote doesn't explain sessions or workflow management and has inconsistent commands and hidden states: these encourage mistakes. So still partly a UI issue. Perhaps the best fix would be to adopt some rshell UI principles and documentation style.

dpgeorge commented 2 years ago

The fs cp command was changed to require explicit : in all cases in 24f1161fe21df146b64a657acadf67e723040636

Support for device-to-device copy (fs cp :a :b) was added in f5fedf4676274dd26d70142b08753873dc1b99bc

CroftAr commented 2 years ago

Yes the function is there but it works back-to-front (versus other fs commands). Compare with the logical and seamless way rshell handles both the device and local host. Maybe mpremote can handle both but the documentation ties most fs commands to the device only.

peterhinch commented 2 years ago

Rshell has a clear concept of layered sessions for REPL commands and rshell commands, making the command language consistent and easy to use.

The modal design imposes a limitation in that rshell commands cannot be scripted. I modified rshell to add a macro facility which makes it more usable in complex projects.

mpremote's modeless approach is inherently scriptable with much more flexibility than mere macros.

The remaining killer advantage of rshell is the rsync command.

CroftAr commented 2 years ago

Can you illustrate please? Perhaps an example in mpremote and rshell with macros, showing how plain rshell falls short. (I'm going mainly by the docs so perhaps the commands do more than I'm aware of.)

peterhinch commented 2 years ago

I have documented macros here with examples.

I have a bash alias for each project which exports a PROJECT environment variable and sets the working directory to the project home directory. My rshell_macros.py script tests the PROJECT envar and populates the macros dict with macros appropriate to the current project. Thus a sync macro issues the necessary rsync commands to update the current project on the target hardware. Similarly the command m run script.py executes a test script on any project, regardless of the different directory structures which affect where script.py is located.

This saves a lot of typing and potential errors (rsync is easy to get wrong) and makes basic maintenance consistent across multiple projects.

If mpremote acquires rsync functionality I think the same thing will be possible with bash scripting, but I haven't yet figured out the details.

CroftAr commented 2 years ago

I follow now: instead of configuring rshell macros for a project you hope to use bash to configure mpremote commands per project, and bash would be more flexible than rshell macros.

Like mpremote, rshell can take commands from its arguments so it can be scripted in bash, but only mpremote can resume a previous session, meaning that it relies on the state saved when its previous invocation terminated.

Using state stored between invocations is awkward in a multi user environment, and so in bash scripting you'd typically store state in the script and pass it to and from each command if you can. Would that work here with a suitably tweaked mpremote?