mu icon indicating copy to clipboard operation
mu copied to clipboard

add circuitpython run support

Open fmorton opened this issue 6 years ago • 66 comments

This pull request is a little deep for someone new to the project, but this is a good example of a "scratch your own itch" sort of change.

It is great to be able to edit files directly on the circuit playground, but it can cause confusion for students learning the discipline of using source code control (like github).

So, this pull request attempts to leave mu operating exactly as before with one exception. If someone is editing a file that is not on CIRCUITPY, it presents the "Run" button that has an implied "save" and then copies it onto CIRCUITPY as code.py. This way, the original source can remain under source code control, but running the code is just a single click.

The challenge with this is to keep the "Run" button from showing when it should not show. For example, if the circuit playground is not connected, the "Run" button should not show. If the file is already located on CIRCUITPY, the "Run" button should not show. This means that at times, the menu bar will need to change dynamically.

One part of this is the addition of a "actions_dynamic" method that always returns False with the exception of the adafruit mode. This prevents unnecessary redraws of the menu bar in all the other modes.

The adafruit actions method adds the "Run" button when appropriate as explained above.

The menu bar is redrawn using the change_mode method, which might be overkill, but was the best way I could see to do it. Open to feedback on a way to do this less intrusively. Something I don't like about this is the menu bar tends to "blink", but I don't see any way around it without adding a lot of complexity.

I did run "make check" with this to keep up with the styling standards but did not add any test coverage for these changes. I've been building software since the 70's, but this is actually the first thing I have done using python, so I still have a lot to learn about python and how things are done. I have been using Ruby for about 10 years, so it is at least not completely unfamiliar.

This was done on Macintosh and has not been tried on Windows.

Thanks.

Frank

fmorton avatar Jun 14 '18 16:06 fmorton

Here is a very basic screencast showing the idea of the pull request:

https://youtu.be/x8B39XJnnvk

The revert on 'untitled' is indeed a mistake.

The actions_dynamic method returns False for all modes except adafruit. If set to True, then the menu bar is redrawn to reflect if the "Run" button should be there or not. This happens through change_mode, even though it is not really changing the mode, but just needs to potentially redraw the menu bar.

fmorton avatar Jun 14 '18 17:06 fmorton

Ah thanks now it begins to make sense! Very much in favour of "scratch your own itch" (as @ntoll knows from my occasionally bizarre PRs :smile: )

My feeling at the moment is that the show/hide of the run button is perhaps a little too "magic". It may be better to have it visible all the time but show a message when there isn't a device attached (À la flash in micro:bit mode), this would also avoid logic like actions_dynamic

I would suggest this is added to the 1.1 milestone as it's a new feature

ZanderBrown avatar Jun 14 '18 20:06 ZanderBrown

Sounds good. I will add the code so the "Run" button is always available, but just provides an error message if a circuit playground is not available. I was trying to avoid changing the user interface.

Also, if the file is already on CIRCUITPY, the "Run" button will be just like the "Save" button.

fmorton avatar Jun 14 '18 21:06 fmorton

Will move away from the magic run button and issue another pull request.

fmorton avatar Jun 15 '18 14:06 fmorton

No need to close this one and open a new one, any work done on this branch would automatically show on this PR :)

carlosperate avatar Jun 15 '18 14:06 carlosperate

Thanks. Reopened.

fmorton avatar Jun 15 '18 14:06 fmorton

This PR adds the "Run" button in adafruit mode without the button showing at different times depending on circumstances. Now it just shows all the time.

All of the actions_dynamic items have been removed to simplify.

So, the "Run" button works just like the "Save" button with the addition of copying the file to CIRCUITPY if the file is not already there and CIRCUITPY is available.

fmorton avatar Jun 20 '18 19:06 fmorton

Hi folks... apologies for the absence... I've been working on getting beta 16 out the door and thank you @fmorton for your contribution.

I concur with @zander that this feels like a 1.1 feature.

Here's the thing, it's a change to the way the UI works in Adafruit mode. Whenever we've made such changes before we've always based them on evidence from user feedback. For example, you'll notice that the "REPL" has been re-named to "Serial" and the functionality slightly changed. This was based on feedback from several users on Adafruit's discord channel collated by @tannewt, discussed in a ticket and we eventually came up with the new behaviour after exploring several possible solutions.

My concern is that it's scratching your personal itch (@fmorton) which may not be an itch for others and there's a danger it might make Mu less understandable / easy to use etc... Put simply, I'll need feedback from several users about how your proposed change to Mu's UI / behaviour help/hinders them.

(I'm firmly wearing my devil's advocate hat here BTW). ;-)

Alternatively, let's step back and think about the problem we're actually trying to address (which is definitely a legitimate problem I can empathise with): when I save my work onto a CircuitPython device, it is not stored in a location which is under source control.

How about an alternative solution to this? For example, in the "admin" dialog we can have an Adafruit tab containing a flag for (I'm making up names here, off the top of my head) "shadow saving". If the flag is set, not only does the save button save the file onto the connected device, but it also saves a "shadow" copy of the file into your mu_code directory. This means the feature is opt-in, has no change to the UI (thus avoiding the hoped-for user feedback) and still solves the problem of saving into local storage under source control. I also suspect, it's going to be easier to implement and test too.

Thoughts on this? As always, I'm open to suggestions, push-back and constructive criticism. This is very much an exercise in exploring the problem space and isn't in any way meant as criticism, IYSWIM! Together, after such explorations, we'll find the simple and most effective way forward in addressing your totally legitimate "itch". :-)

ntoll avatar Jun 21 '18 13:06 ntoll

(Sorry @zander, @ntoll meant @zanderbrown aka me)

ZanderBrown avatar Jun 21 '18 14:06 ZanderBrown

I tend to agree with you about the UI change. That is why I had originally done it so that the "Run" button only showed up if the file was not on the device and the device was available. But, I also agree that the magic "Run" button that appears and disappears depending upon context was a little too magical.

Adding a tab to the admin section sounds like a good solution that makes it available but as an opt-in feature so the user can decide. If they want the run button, they will have to go check the box and it will appear.

I am guessing I can figure out how to do that without too much digging, so will proceed this way if that makes sense to everyone.

fmorton avatar Jun 21 '18 19:06 fmorton

Just to play "God's advocate" (hmm... that seems like a big responsibility), for the sake of argument?

Having a "run"-like button in Adafruit's mode probably unifies the UX with the other modes:

  • Python has "run"
  • PyGame has "play"
  • micro:bit has "flash"
  • Any future generic MicroPython mode will also need an action button as not all use USB MSD.

And the adafruit doesn't have any "action" button that clearly states that you are going to run your script.

If I was an absolute complete beginner, how would I know that I have the save the python file into the adafruit device to load it? Just by looking at Mu I would not be able to guess this. It is a very easy concept to anybody that has used the device already, but maybe not so much for first timers.

If you remember at the beginning with the microbit, the concept of having to save a hex file into the USB drive was quite easy to grok by some people and others only really got it after it was demonstrated to them. In fact, having a one-button clear action was one of the big advantages of Mu over the online editor.

How about an alternative solution to this? For example, in the "admin" dialog we can have an Adafruit tab containing a flag for (I'm making up names here, off the top of my head) "shadow saving". If the flag is set, not only does the save button save the file onto the connected device, but it also saves a "shadow" copy of the file into your mu_code directory.

I think I would always be confused as to which one of the two files is my latest version, or it might not be clear to a user (although this might just be me and my paranoia of stale copies). But it is an advanced option for advanced users, so they should be aware of how the feature works and that's fine.

This means the feature is opt-in, has no change to the UI (thus avoiding the hoped-for user feedback) and still solves the problem of saving into local storage under source control. I also suspect, it's going to be easier to implement and test too.

To be fair this solution doesn't really help the original advanced situation, which is having the script in a specific location, like a local git repository, and wanting to load it into the device. Saving the script in the mu_file directory would just keep a copy there rather than having a copy that could live anywhere. The only way that would work is by having an option in the settings that tracks shadow script locations for individual tabs, and that would be overly complex for an advance feature that perhaps is out of scope in this application.

While the original problem is an advance feature that could or could not be out of scope for the project, or the app UX, I think that having an "action" button does help with usability/discoverability of loading a script in the Adafruit mode. In addition we get the bonus that it offers the flexibility to work on scripts saved anywhere in your computer (like a git repo) allowing advanced users to do more, without having an odd option in the settings that could be hard to explain/understand.

While I wasn't too sure before, I think writing this response has convinced me a bit more that this could be a good addition, and it is worth noting that changing a button before v1 is preferable vs after v1.

Thoughts?

carlosperate avatar Jun 21 '18 19:06 carlosperate

I will defer to all of you regarding which approach is best.

I have the "admin" approach finished and will check-in if that is how you all think this should go.

In any event, consistent with the other modes, the "Run" button should be positioned before the "Serial" button to align with the other modes. The previous discussion caused me to notice this difference.

fmorton avatar Jun 21 '18 20:06 fmorton

I went ahead and checked-in the admin support for the adafruit "Run" button. I will be out-of-touch for a couple of weeks, so I went ahead. It is easy enough to take out if that is what would all want to do.

fmorton avatar Jun 25 '18 14:06 fmorton

Green check mark. Fixed a couple of minor test issues.

fmorton avatar Jun 26 '18 20:06 fmorton

Hope you consider this for the next release now that it is simpler and as others have pointed out, is consistent with the other modes. I have to think this is a common need for students like ours.

fmorton avatar Jul 02 '18 16:07 fmorton

Updated this PR. It is now a compromise that I think is good. The interface is unchanged normally. But, if you go to the "Adafruit Settings" on the Administration panel and set the "Enable the Run Button..." checkbox, the Run option appears on the main menu bar. The Run button is the same thing as Save, except it copies the file to CIRCUITPY if the device is available. Best of both worlds. Simple if you leave it alone and will cause no confusion for current students. If you would rather use source code control, you can use the Run button to run that source.

fmorton avatar Sep 20 '18 18:09 fmorton

Added some more. Could use some help looking why the travis ci build fails.

This addition continues along the same line. There is now a second preferences checkbox under the adafruit tab. If it is selected, and if at the same location as the main source file, there is a "lib" directory, any library located in that directory gets copied over to CIRCUITPY/lib when you select the "Run" button. File dates are checked to keep from updating the same library file over and over.

We are using this now with kids at our makerspace, so it is being "kid tested".

fmorton avatar Oct 18 '18 17:10 fmorton

What do you want to do with the PR?

fmorton avatar Dec 26 '18 05:12 fmorton

Please let me know if you plan to include this in the 1.1 release.

fmorton avatar Mar 31 '19 03:03 fmorton

I am getting requests from people wanting to use this pull request. Can one of you direct me to what I need to do to generate a macos install package?

Thanks.

fmorton avatar Feb 04 '20 01:02 fmorton

I am getting requests from people wanting to use this pull request. Can one of you direct me to what I need to do to generate a macos install package?

@fmorton, here's the gist:

  • You need to be developing on macOS.
  • Have the current "master" installed in editable mode, with the dev extras:
    • pip install -e .[dev] under a virtual environment of your choice.
  • Maybe locally merge the changes you're proposing in this PR.
  • Run make macos at the root of the repository.
  • This should produce a mu-editor.app macOS application bundle under the macOS directory.

Things to keep in mind:

  • Such output is neither signed nor notarized (as required?) by the more recent macOS Catalina.
  • Deploying it to other macOS computers may require some "authorization" fiddling.

Let me know how this works for you. :)

tmontes avatar Feb 04 '20 09:02 tmontes

Thanks for your response.

Your suggested steps did generate the mu-editor.app with one addition. There was a conflict reported as:

botocore 1.14.9 has requirement docutils<0.16,>=0.10, but you'll have docutils 0.16 which is incompatible.

so installing 0.15 of docutils resolved this issue.

However, now when attempting to launch the application, the system log reports:

(mu.codewith.editor.mu-editor.49044[13322]): Service exited with abnormal code: 1

Attempting to start the application from the command line using:

/Applications/mu-editor.app/Contents/MacOS/mu-editor

generates this error message:

/Applications/mu-editor.app/Contents/Resources/python/bin/python3: No module named mu_editor

It does feel like an authorization issue as you alluded to, but so far, I have not been able to discover a resolution.

Glad to hear and try any suggestions.

Frank

On Feb 4, 2020, at 4:31 AM, Tiago Montes [email protected] wrote:

I am getting requests from people wanting to use this pull request. Can one of you direct me to what I need to do to generate a macos install package?

@fmorton https://github.com/fmorton, here's the gist:

You need to be developing on macOS. Have the current "master" installed in editable mode, with the dev extras: pip install -e .[dev] under a virtual environment of your choice. Maybe locally merge the changes you're proposing in this PR. Run make macos at the root of the repository. This should produce a mu-editor.app macOS application bundle under the macOS directory. Things to keep in mind:

Such output is neither signed nor notarized (as required?) by the more recent macOS Catalina. Deploying it to other macOS computers may require some "authorization" fiddling. Let me know how this works for you. :)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/mu-editor/mu/pull/485?email_source=notifications&email_token=AABEM2WVN543EEIVY4TE443RBEYYTA5CNFSM4FFBDBUKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEKW5YKI#issuecomment-581819433, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABEM2UN3AIDMZ5I5LEFJJ3RBEYYTANCNFSM4FFBDBUA.

fmorton avatar Feb 04 '20 20:02 fmorton

/Applications/mu-editor.app/Contents/Resources/python/bin/python3: No module named mu_editor It does feel like an authorization issue as you alluded to, but so far, I have not been able to discover a resolution. Glad to hear and try any suggestions.

Hmmm, that looks strange and unexpected. Feels like something bad happened during the packaging process, but I wouldn't know, without more information. Have you noticed issue #1000 where a user was having issues much like yours -- packaging a macOS Application Bundle -- where I shared similar advice that seemed help?

On my mind:

  • Which version of Python are you using to create the macOS Application Bundle? Shipped/installed from where? (my success stories are based on CPython 3.6 from www.python.org, none other -- I guess 3.7 would work, 3.8 looks like it won't, yet)
  • Could reviewing #1000 give you a hint to help you out?

Let me know. :)

tmontes avatar Feb 06 '20 10:02 tmontes

Thanks to @DaveSprague for working through generating a macos app to use this PR before it gets integrated with the main branch. Here are his instructions for generating the app using python 3.6. Read all the way to the end, particularly if you are using Catalina:

git clone https://github.com/mu-editor/mu.git cd mu python3 -m venv .venv

//{note: if you name your virtual env something other than ".venv" then edit setup.cfg and add that folder to the ignore list for flake8}

source .venv/bin/activate pip install --upgrade pip pip install -e ".[dev]" pip uninstall botocore pip install botocore git pull origin pull/485/head make macos

// now move the mu-editor.app file from the macos folder to the Mac applications folder (or you an just run it from the macos folder if you want). If you're running Catalina then you might have to give the app permission to access the CIRCUITPY volume. To do that go under Settings | Security and Privacy, Select the Privacy tab at the top, in the left column select "Full Disk Access" and then on the right side click the box next to mu-editor to give it permission. If those options are grayed out, you need to select the padlock in the lower left of the dialog box and enter you system password before you can chance the settings.

fmorton avatar Feb 07 '20 22:02 fmorton

A build for 1.1.0-alpha.2 with this pull request included is available to place in your /Applications directory at:

https://cdn.neighborhoodmakers.org/releases/mu-editor.app.tar.gz

fmorton avatar Feb 09 '20 20:02 fmorton

Should I give up on this pull request?

fmorton avatar Feb 01 '21 03:02 fmorton

Should I give up on this pull request?

I'd like to see this feature, in some form, added to the Mu editor. Is there a plan to implement this in v1.1 of Mu?

I help teach coding to students using Arduino and I'd like to work with them on to Circuit Python but the current way Mu works, where they would have to maintain two copies their code with different names (one on the Adafruit board and the other on the computer's file system) seems overly complicated. The other alternative is to keep the different programs they're writing all stored just on the Adafruit board but then do you fix bugs by editing the named version and then copying it over to the code.py file or do you just fix bugs in the version that's named code.py and then try to remember to copy that back to the named version?

If you're building a specific project for an board then just having one code file, named code.py, perhaps isn't that big a deal but when you're teaching students to code then over the process of a few weeks you might be writing 10 different programs (blinking LED, motor control, servo control, ...). It would be nice if Mu had a user model more similar to Arduino and other coding environments where you can manage a set of programs, named appropriately, and then run one of those programs easily.

It seems like a much simpler user model to me is to "hide" that detail of renaming to code.py from the Mu user. They write programs stored on the computer or on the adafruit board and give them names that describe what they do and then they press the "run" button which causes Mu to make a copy of that code file called "code.py" on the adafruit board in order to run it.

DaveSprague avatar Feb 01 '21 20:02 DaveSprague

Agree completely. I am willing to get the code up-to-date again if it will be included in a release. This sort of thing is required when managing a classroom and teaches a good habit of using source code control. I am happy to take input too if there is some reason this pull request is not appropriate for some reason I do not know.

fmorton avatar Feb 02 '21 04:02 fmorton

Can this get in the beta? I can catch it up on Monday.

fmorton avatar Feb 20 '21 04:02 fmorton

Hi Frank, I think perhaps the reason this issue has been stalled a bit is that there's no real consensus about what way to go. Normally we would have a discussion in an issue, try to agree on the best way to solve that issue before any PR's are made. In this case, it went the opposite direction. There's been some discussion here, with various alternative solutions, but it seems any consensus was never reached.

I think we should revive that discussion. I'm sorry you've used so much time implementing this and waiting already, but we should try to achieve consensus before we merge.

There are at least two different suggested solutions, where you have implemented one of them, and as the developer of the similar ESP-mode I will just add a third suggestion as well. Here's the three different suggestions:

  1. (copy-to-device) Your solution is the addition of a new "Run"-button, which makes the CircuitPython-mode behave similar to micro:bit-mode in the sense that you can write your code locally in mu_code, and when you want to run the code it executes by copying the file to CIRCUIT_PY/code.py
  2. (shadow-copy-from-device) @ntoll suggested that instead of adding a new Run-button, instead the Save-button should be modified for CircuitPython-mode, such that when you save to the device, the same file is mirrored to a location in your mu_mode directory.
  3. (run from mu_code) I will add the suggestion that CircuitPython could function like the ESP-mode (and thus also the LEGO-mode, Pyboard-mode, and Raspberry Pi Pico-modes), where you develop your solution locally, and then add a new Run-button, which doesn't write anything to the CircuitPython device, but just executes the code directly on the CircuitPython-device. If the user wants to write the program to the CircuitPython device, he or she, would have to copy over the file and overwrite the existing code.pymanually. That will make users save their projects to mu_code, like for any other Mu-mode.

So, that's the current set of possible solutions. Now we can share our opinions I guess.

My opinion is to go with either 1. (copy-to-device) or 3. (run from mu_code), which will then introduce this new UI-component (the Run-button), which would require all teachers etc. to change their screenshots and teaching material. I know that would require changes for a lot of people, but I just can't see how 2. (shadow-copy-from-device) would actually work. When you develop a CircuitPython project, you almost always save your file as code.py directly on the device, if we create a "shadow-copy" setting for Mu, where would that code.py be copied to? How would it know that at one time you were working on project X and another time working on project Y? Should it just be one big mu_code/CircuitPython/ directory with timestamped code.py files for everytime a user tested their project on the device?

Perhaps my suggestion number 3. (run from mu_code) goes too much against the ideas of CircuitPython, I'm not sure, I haven't really used CircuitPython-devices, as I'm more of an ESP32/ESP8266 user myself. But if you want to test how it works, you could try hooking up your CircuitPython-device and just switch to the ESP-mode, and try it out.

dybber avatar Feb 20 '21 08:02 dybber