add circuitpython run support #485
Conversation
mu/logic.py
Outdated
|
|
||
|
|
||
| def get_tab(self): | ||
| return get_view(self).current_tab |
There was a problem hiding this comment.
Attach these to the relevent class for things like
self.get_view().current_tab
mu/logic.py
Outdated
| @@ -919,7 +936,7 @@ def check_code(self): | |||
| if tab.has_annotations: | |||
| logger.info('Checking code.') | |||
| self._view.reset_annotations() | |||
| filename = tab.path if tab.path else _('untitled') | |||
| filename = tab.path if tab.path else 'untitled' | |||
There was a problem hiding this comment.
Why is this reverted? (probably an accident)
mu/modes/adafruit.py
Outdated
| @@ -53,6 +55,9 @@ class AdafruitMode(MicroPythonMode): | |||
| (0x239A, 0x802E), # Adafruit CRICKit M0 | |||
| ] | |||
|
|
|||
| def actions_dynamic(self): | |||
There was a problem hiding this comment.
What does this signify?
|
Here is a very basic screencast showing the idea of the pull request: 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. |
|
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 😄 ) 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 I would suggest this is added to the 1.1 milestone as it's a new feature |
|
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. |
|
Will move away from the magic run button and issue another pull request. |
|
No need to close this one and open a new one, any work done on this branch would automatically show on this PR :) |
|
Thanks. Reopened. |
|
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. |
|
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 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". :-) |
|
(Sorry @zander, @ntoll meant @ZanderBrown aka me) |
|
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. |
|
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:
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.
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.
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 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? |
|
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. |
|
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. |
|
Hi @ntoll Thanks. I should be able to followup on whatever needs to be done. Nice to see all the checks passing, too. |
|
Has this been merged into the master yet or is there a plan to do so? I really want to have this feature available for a high school class I'm teaching where we'll be using circuit python. |
|
I went to generate a new macos release, but get this error from black complaining about uflash.py: black Oh no! 💥 💔 💥 |
|
Which version of black are you running? Make sure the version you have is within the versions listed in the setup.py file. If it is, it'd be good to know which one it is in case we need to change the version range. Safest bet would be to get the latest version within the setup.py range, as that's what CI would be running. |
|
Using python 3.9.7 and black 22.8.0. |
|
I tried to go back to black 22.1.0 but ran into a problem on my system that requires 22.3.0 to run black at all. |
|
I locally removed the uflash.py file as a trial and did another "make macos". Ran into another black error with tests/interface/test_panes.py. |
|
The rule in setup.py is We'll update at some point (and with that we'll have to reformat any files that fail with the newer version), but doing so means setting the min version to Mu is currently configured with Python 3.8 as the max supported version, and the pinned versions of PyQt5 don't have (or at least didn't have) wheels for Python 3.9+, so there must be other changes to your setup.py already (or it was pip installed using the flags required to ignore these rules). The easiest way to get it running might be to create a virtual environment with Python 3.8 and the dependency versions listed in setup.py. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| def test_get_pathname(): | ||
| mock_func = mock.MagicMock(return_value="CIRCUITPY") | ||
| with mock.patch("mu.logic.get_pathname", mock_func): | ||
| assert mu.logic.get_pathname() == "CIRCUITPY" |
Check failure
Code scanning / CodeQL
Wrong number of arguments in a call Error test
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