-
-
Notifications
You must be signed in to change notification settings - Fork 8.3k
tools/mpremote: Make resume behavior the default, remove resume command #17485
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
base: master
Are you sure you want to change the base?
tools/mpremote: Make resume behavior the default, remove resume command #17485
Conversation
Code size report:
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #17485 +/- ##
=======================================
Coverage 98.54% 98.54%
=======================================
Files 169 169
Lines 21941 21946 +5
=======================================
+ Hits 21621 21626 +5
Misses 320 320 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This is prompted by discussions started in #17322 (comment) For reference this was built from just three prompts in a new Claude code session:
I started with that above to encourage Claude to review the code around the area of interest, I then followed up with a more specific request:
Which returned:
So I asked:
and that was enough to get the PR up here. Of note; it gets commits and pr template basically right thanks to my current: TODO test on hardware to ensure it behaves as expected, after which I'll mark it as ready for review |
Changes Python session state preservation to be the default behavior. Previously, mpremote performed auto soft-reset between command sessions, clearing variables and imports. Users had to explicitly use 'resume' to preserve state. This change: - Sets _auto_soft_reset = False by default in State.__init__() - Removes the 'resume' command entirely - Updates tests to reflect new default behavior Users wanting the old auto soft-reset behavior can now explicitly add 'soft-reset' to their command sequences. Breaking change: Existing scripts expecting auto soft-reset will see different behavior. Migration: add explicit 'soft-reset' commands where clean state is required. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Andrew Leech <andrew.leech@planetinnovation.com.au>
5608292
to
901cf0d
Compare
Is this a meaningful thing to add? A tool is not an author and a "noreply" email isn't much use for posterity. Edit: To engage with the change itself - if you wanted a repl wouldn't you just use a repl? I would argue that state persisting between individual mpremote commands would be the more surprising outcome, since mpremote is - to my understanding - intended to get one thing done and then clean up after itself. |
I'm in favour of this change. I think it makes
Also, in the cases where the connection is lost on soft reset (eg with a dynamic USBDevice, or WebREPL) it makes sense for a soft reset to be an explicit command, because you usually don't want it. |
I use mpremote a lot through subprocess run in mpflash, micropython-magic, end other tools. I see resetting as a side effect that should only happen if requested. |
I'm very open to discussions about this, at the moment I'm just trying to be very up front about my use of the tool. I'd kinda argue the tool was the primary author and I'm just the reviewer. Sure I provided the description of the change I wanted, but I never wrote a single line of code here. Regardless, the coauthor tag is recognised by GitHub such that it shows prominently that Claude was used which I think it's a good thing to be open about; though it's true I'm still the person responsible for submitting the change request. |
99% of my usage of mpremote is to attach to a device already running code, with aiorepl running in the codebase. It's very rare I want it to reboot the device on me, unless I actually send the reboot or bootloader command explicitly. I'm generally using micropython to build products, so have applications running from main pretty much all the time outside of small research / feature development spikes or separate micropython pr development efforts. So your suggestion of it being surprising that a device would keep running between mpremote calls is curious to me; I'd be interested to hear more about your workflow that lead to this expectation? I don't want to make mpremote more confusing for people! |
I think we come at this from wildly different angles and I spend 99% of my time deep in the weeds writing C modules, and most of my higher level development time ardently dogfooding Thonny. My uses of mpremote start and end with
Feels like a delicate balance between an advertisement for Claude and an admission that it's useful to know what it's touched just in case it proves to be so radioactive that it has to be chopped out of the codebase 😆 It's also a tacit approval of AI that could open the floodgates to much less thoughtful use of it than yours (uh that ship might have sailed without my noticing, to be fair). Suffice to say my takes on AI contributions to code are not optimistic. (understatement of the year, since I don't believe even thoughtful use is productive beyond the most crude and short term measures) Fundamentally Claude can't really "own" a commit in any meaningful sense. AIUI It doesn't have institutional knowledge beyond that enshrined in the code itself, nor can it consistently or reliably hold a viewpoint or belief and advocate for a particular decision. Even if I fed it your/its original rational/analysis/prompt. Thus it functionally cannot be an author or a meaningful, discrete part of the fleshy machine that drives forward a FOSS project. That is to say- If I were to question Claude about this change and posit an alternative, I suspect it would immediately capitulate to whatever I thought was best 😆 If I were to question you... well, you have successfully given rationale, and argued your corner. Anyway, TLDR: there are probably better places for this discussion, or perhaps conversations I have already missed. As you were! |
I was the same only a few months ago, my belief was they were mildly helpful in a very narrow set of use cases. Since I got access to Claude Code my entire professional way of working has been turned upside down, this tool is redefining my job role and more than doubling my productivity and quality of work.
This is a useful angle to think of certainly. At work the crucial point of view is that regardless of how instrumental AI might be in producing a piece of work I, and the company, have to take ownership it from a responsibility point of view and have to stand by it being as good (if not better) then work I would have done the manual way.
While yes it's off topic to the code of the PR I did prompt this discussion in my first comment... though maybe there should be a formal discussion started elsewhere to discuss some policies around appropriate declarations of AI use that's informative without being just advertisement. |
I was getting unsure whether a reset before
Avoiding the implicit reset for the cases where connections are broken is the one I keep coming back to as a stronger argument for this change; it would be nice to change soft reset enough to never break connections but these ones in particular would be really hard to achieve. |
Another argument for making resume the default: currently if you do:
then that works as expected, it sets and prints But if you run the exec's separately:
then it doesn't work because the second time it does a soft reset and loses all variables. If we make resume the default, then both the above examples work the same. I think |
Summary
Changes Python session state preservation to be the default behavior in mpremote. Previously, mpremote performed automatic soft-reset between command sessions, clearing variables and imports. Users had to explicitly use the
resume
command to preserve state.This change makes the "resume" behavior the default and removes the
resume
command entirely. Users wanting the old auto soft-reset behavior can now explicitly addsoft-reset
to their command sequences.Example behavior change:
Testing
test_resume.sh
test to reflect new behaviorsoft-reset
commands replicate the old default behaviorTrade-offs and Alternatives
Benefits:
resume
commands in workflowsBreaking change considerations:
soft-reset
where clean state is requiredImplementation details:
State.__init__()
to set_auto_soft_reset = False
by defaultdo_resume()
function and command registrationsoft-reset
command continues to work as beforeThis provides the same functionality as before but with inverted defaults, making the more commonly desired behavior (state persistence) the default while still allowing users to get clean state when needed.