add support for web workflow

[vladak]

This change introduces the support for web workflow. Tested against ESP32 V2's with 8.0.0 beta 4 (cannot use 8.0.2 due to adafruit/circuitpython#7636). pytest of the pre-existing tests passes. I can be certainly persuaded to add web workflow specific tests.

[FoamyGuy]

@vladak Thanks for working on this! It looks like really neat functionality. Sorry, it hasn't been reviewed yet. I'm trying to take a look now and try it out.

Do you have an example of a command that can be used to test this? I have an ESP32-S2 that is setup and working with web workflow, but I'm not sure how to get circup to see it. based on the code diff it looks like --path is to be used for it, but I haven't figured out the proper syntax.

I tried these:

circup install chime --path http://192.168.1.119
circup install chime --path http://cpy-9573b2.local

Both report this error:

Could not find a connected CircuitPython device.

[FoamyGuy]

I merged main and resolved the conflicts to get this back to passing.

I did figure out a proper command for testing this:

circup --host 192.168.1.119 --password secret install cedargrove_chime --py

I was able to successfully install libraries with that command.

I think we still need a bit more testing on USB based devices to make sure none of the new changes impact people using the library in the up to now at least more primary mode of operation. I did a bit, but only minimal, I intend to try some more in the coming week.

There are also some place(s) in the functionality that are assuming USB based device that fail when used with web workflow. The one I found is the --auto command.

When you try like this it fails with an error about not finding the auto file

circup --host 192.168.1.119 --password secret install --auto --py
> Auto file not found: http://:secret@192.168.1.119/code.py

I think that the URL would need a /fs in it for the web workflow.

You can work around that by using the --auto-file argument like this, but it still fails with a similar error:

circup --host 192.168.1.119 --password secret install --auto --auto-file fs/code.py --py
> Auto file not found: http://:secret@192.168.1.119/fs/code.py

That makes it have the proper URL, but it still fails due to the logic here: https://github.com/vladak/circup/blob/web_workflow/circup/__init__.py#L1673 attempting to look on the local storage to find the device. It would need an if statement with some logic to handle the way that web workflow will need to check for the files existence, there may be other adjustments needed beyond that as well, but we'll have to make this one in order to tell.

@vladak if you're interested and willing to work on the adjustments needed for web_workflow with --auto argument you can add new commits to this branch. If you're unable to work on it at this time I'll try to circle back around and make the necessary changes and push them to this branch.

I think once auto is functional and we get a bit more testing on the USB mode to ensure everything is still good there then this will be ready to merge.

[vladak]

@FoamyGuy I'd appreciate if you could finish the changes. I can probably do some USB based testing.

[FoamyGuy]

This is looking good to me now in it's current state.

I have successfully tested the following contexts:

• usb individual library install
• usb install --auto
• web individual library install
• web install --auto

Also installed successfully from both community bundle and adafruit bundle.

[tannewt]

Would you be willing to refactor this a bit further? Right now it's using a kwarg or url scheme to determine how to behave. I think it'd be better to have two "backend" classes that implement the functions needed by the circup code. Then, you'd instantiate file system or web workflow backend and use it throughout. This sets us up for BLE circup support too.

Thanks for taking this on!

[rsbohn]

• TTGO T8 ESP32-S2-WROOM
  LILYGO [SN:C7FD1A109BC9]
  COM28 (REPL)
  D:\ "CIRCUITPY"

Ran this PR code, got: Not supported URL scheme: d.

[rsbohn]

# __init__.py:707
        old_mode = ctypes.windll.kernel32.SetErrorMode(1)
        try:
            for disk in "ABCDEFGHIJKLMNOPQRSTUVWXYZ":
                path = "{}:\\".format(disk)
                if os.path.exists(path) and get_volume_name(path) == "CIRCUITPY":
                    device_dir = f"file:///{disk}:/"
                    # Report only the FIRST device found.
                    break
        finally:
            ctypes.windll.kernel32.SetErrorMode(old_mode)

Not a complete solution:
OSError: [Errno 22] Invalid argument: '/D:/boot_out.txt'

[FoamyGuy]

Effort to refactor is on this branch: https://github.com/FoamyGuy/circup/tree/web_workflow_refactor

When it's ready I'll either merge it to this PR branch, or make a new PR if there are issues with that.

Current status:

• Minimal testing successful on Linux PC for both USB and Web devices, installing individual libraries, update, uninstall, and install --auto.
• Known issue with windows for USB devices info in above comment. This was tested on the version in this PR branch (as of this comments writing) which doesn't have the backend refactor in it yet, but I suspect the same issue will be present in the refactored version and still need to be addressed
• tests need lots of work, there are many pylint failures and very likely it will fail to run or produce meaningful output in it's current state. My current plan is to create a MockBackend that can be utilized by the test script.

[tyeth]

@FoamyGuy was on stream today in this branch https://github.com/FoamyGuy/circup/tree/web_workflow_inprogress
I said I'd have a poke on windows. Now having poked I also spotted there was a bit of requests timeout set before by Neradoc so I made the timeout command line argument / parameter also update that value.
Added free space checking although untested with too little space (simple disk space check or use the json['free'] from lib url) compared to the module it's currently copying. This probably only works on files, I didn't check a folder based module's space calculation and really should have (mental note / TODO -- Also need to add a lot of the arguments to the readme).

My branch has a couple of commits, mostly the os.path.sep usage, and one change where it calculates the module name, so could do with a retest on Mac/Linux. I did check once on wsl (mainly for free space calc) but part way through and I haven't got the mount point only web support in wsl.
https://github.com/tyeth/circup/tree/timeout-reuse
Comparison of the commits on top of the web_workflow_inprogress branch:
FoamyGuy/circup@web_workflow_inprogress...tyeth:circup:timeout-reuse

Also came up trying mDNS, and foamyguy had a good suggestion:

23:50]Tyeth: just playing @FoamyGuy#2825 and found it does actually resolve circuitpython.local, but because the device doesn't serve a site there it reports unauthorized and the real device name (a bit confusing but correct and address given is the device .local e.g. http://cpy-ecead8.local/ )
Instead of using circuitpython.local you then switch to the address it reported as unauthorized, and that's a hacky way of it working over mdns e.g.

circup --verbose --host circuitpython.local --password password --timeout 20 install --auto --py
# requests.exceptions.HTTPError: 401 Client Error: Unauthorized for url: http://cpy-ecead8.local/fs/lib/

circup --verbose --host cpy-ecead8.local --password password --timeout 20 install --auto --py   
# this works, but you can't guess this address, got to use browser or above or something else

[23:54]foamyguy#2825: Theoretically we could make --host optional and if omitted have it send the first request to http://circuitpython.local/cp/version.json then start using the host name that is returned in that file.

[tyeth]

I stuck in the folder space calculation too, after testing with fonts which are folder based, and the output looks like this to a normal user (click.echo calls, also logging, but that's silent in console when debugging unless verbose mode is on - still logged to a file though)
.

I chose to not exit after a diskspace failure, as that allows more modules to be installed after ignoring an obscenely large library or asset like images/fonts/videos.

[tyeth]

Have since found the log file, and mines 162mb, so I've changed it to a 10Mb rolling log file with no backups. Feels like we should probably tell the user it exists (it does with verbose flag, if you catch it before terminal scrollback is lost), but then again most wont want to know until there's a problem.

[FoamyGuy]

@tyeth Does circup install work via DiskBackend for you on Windows with the current code in that branch?

Was the change to this line necessary to make circup work on Windows? https://github.com/tyeth/circup/blob/d5d8ed607d6cae8882b31fa2e8cb29c6ab035e34/circup/backends.py#L633

I'm noticing that this seem to break the DiskBackend for Linux or at least on my system. I added several print statements and found that the path resulting from this line doesn't lincude the path to the actual drive so instead it comes out as just /lib

self.device_location = device_location
print(f"self.device_location: {self.device_location}")
print(f"{(self.device_location, os.sep + self.LIB_DIR_PATH)}")
self.library_path = os.path.join(self.device_location, os.sep + self.LIB_DIR_PATH)
print(f"lib path in init: {self.library_path}")

The above code prints this:

device_location in init: /media/timc/CIRCUITPY
self.device_location: /media/timc/CIRCUITPY
('/media/timc/CIRCUITPY', '/lib')
lib path in init: /lib

The path seperators and joining issues in here have turned my brain slightly to mush and I am not certain of my own judgement on it really, but it looks like maybe this should either use os.path.join() OR include the path seperator with os.sep or "/", but shouldn't have both of those things right?

[FoamyGuy]

@tyeth I do think perhaps the os.sep in that line was extra and causing trouble, at least in my environment. I removed that with this commit: FoamyGuy@1f8443f and it does work now for USB connected device.

I have merged timeout-reuse with my one commit on top with this change into web_workflow_inprogress.

My intention is to do some cleanup and whatever tweaks are needed for pre-commit today and then get everything pushed into this PR branch.

[tyeth]

Just to confirm tested on windows, before and after the change on Disk backend fine. Also tested host mode.

[FoamyGuy]

I think this is ready for another look when you have a chance @tannewt

I was able to remove the Web vs. Disk logic from Module constructor entirely, it turned out to be a workaround for a different bug that I found and fixed which removed the need for that stuff.

The latest commits have removed all of my debugging prints and several formatting and pylint changes to get the actions passing.

I did find the source of the repeated requests to get versions back to back. That has been fixed by changing is_device_present() to use different logic for WebWorkflow.

I did also add some logic for when the host is circuitpython.local it will GET http://circuitpython.local/cp/version.json and then use the hostname found in there going forward.

I tested the current version successfully in these scenarios an a ESP32 S3 TFT:

• USB install and uninstall
• USB list and update
• WEB install and uninstall
• WEB list and update

[dhalbert]

I see you did a bit of refactoring into multiple files. Breaking up __init__.py would be great but I can understand if you don't want to do it for this project.

[FoamyGuy]

I see you did a bit of refactoring into multiple files. Breaking up __init__.py would be great but I can understand if you don't want to do it for this project.

I would prefer to do further refactoring of init.py separately from this PR and the webworkflow feature if possible so that any issues that pop up can be more easily isolated to web workflow vs. init.py refactoring.

[tannewt]

Thank you so much for your persistence on this! It looks good!