Skip to content
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

Should add checks for boot_out.txt #2263

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

LeonHagenaarsKeus
Copy link

Should fix #2260 please point me to documentation in case I am missing stuff.
This is my first github pull request, so please be kind.

@carlosperate
Copy link
Member

carlosperate commented Jun 11, 2022

Hi @LeonHagenaarsKeus, thanks for the PR!

When does the boot_out.txt file get created? The reason I ask is because I'm not sure if this is a reliable way to to detect if it's a CircuitPython board. Is it always created in all CircuitPython ports?

@dhalbert
Copy link
Contributor

dhalbert commented Jun 11, 2022

boot-out.txt will definitely exist after CircuitPython hard-resets. If it does not exist, CircuitPython creates it, and logs information about the firmware. If it exists and its contents would be the same as what is already in the file, it is not rewritten (to save flash wear). It also contains output (including errors) from boot.py, which runs before USB is available.

@carlosperate
Copy link
Member

boot-out.txt will definitely exist after CircuitPython hard-resets.

Ah, right, I see, thanks! But does it exist in a cold start?

@dhalbert
Copy link
Contributor

Ah, right, I see, thanks! But does it exist in a cold start?

Yes, if it does not exist on a hard-reset or a power-up (plug-in), it will appear

@LeonHagenaarsKeus
Copy link
Author

It took some time to get the mu development environment actually up and running, but I've tested it now under windows.

It works.

The test is a 'fallback', so if the prior tests (i.e. testing for CIRCUITPY as device label under windows and checking for CIRCUITPY or PYBFLASH under linux/OSx) find nothing, this test is done, otherwise the prior tests return true.

Separate but related question: Why does Linux/OSx check for both PYBFLASH and CIRCUITPY, where Windows only checks for CIRCUITPY?

@LeonHagenaarsKeus
Copy link
Author

https://github.com/adafruit/circuitpython/blob/main/main.c#L710 contains the checks for writing the boot_out.txt.

Looking at it, I realize why @dhalbert was so convinced this works.

@dhalbert
Copy link
Contributor

Why does Linux/OSx check for both PYBFLASH and CIRCUITPY, where Windows only checks for CIRCUITPY?

I don't know. PYBFLASH is what MicroPython shows. Is this path also used for MicroPython mode?

@LeonHagenaarsKeus
Copy link
Author

I don't know. PYBFLASH is what MicroPython shows. Is this path also used for MicroPython mode?

This path is only available in CircuitPython mode, if I understand Python's inheritence model correctly. CircuitPython 'extends/inherits from' the MicroPython mode.

@LeonHagenaarsKeus
Copy link
Author

Hello @carlosperate, what can I do to help this merge request along?

@carlosperate
Copy link
Member

carlosperate commented Jun 22, 2022

Hi, first all of all, thanks for looking into this issue and preparing a PR!

Thinking about this I am a little bit apprehensive to add something that does a file access to all available volumes. This might be fine for most users, but I worry about computers with things like networked drives, which can be found in school networks and similar. In these cases drive access can cause all sorts of errors and/or long delays. I've had first hand experience with multiple apps completly looking up when the computer I was using had network drives, and introducing this kind of thing in Mu makes me a bit nervous.

Would there another way to detect it's a CircuitPython device without accessing a file? Maybe any other volume properties that could be check via win32api?

@LeonHagenaarsKeus
Copy link
Author

Would there another way to detect it's a CircuitPython device without accessing a file? Maybe any other volume properties that could be check via win32api?

I'll look. What would be the equivalent api for linux/mac? The current check already has differences, I wouldn't want to add to the divergence of functionality between the two.

@Neradoc
Copy link
Contributor

Neradoc commented Jun 26, 2022

Reading boot_out.txt on the drive can be disruptive since it might cause an auto-reload, I don't actually advise doing that. In addition to the other reason mentioned above.

I did some work in discotool to be able to detect and associate drives and serial port with USB devices. That part of the code in the "usbinfos" submodule (and can be used as a library). Each platform needs its own code and it's... a little hacky, and could probably be rewritten better by someone knowing more about the underlying APIs on each platform. Also I didn't want to use pyusb which requires libusb (which is not installed by pip) which might have helped sharing more code between platforms. It might not be compatible with as many versions of the OSes as Mu targets.

Basically it finds USB devices, and travels the USB hierarchy to associate together the serial ports and the USB Mass Storage device (along with identifiers like the serial number, VID, PID, constructor and name) as they will be part of the same sub-tree. It also has to associate the mounted partition with the storage device.

On windows I use the win32api and wmi modules, on linux pyudev and psutil, on mac psutil and the system_profiler tool.

@LeonHagenaarsKeus
Copy link
Author

After looking at the device id's in the windows device manager, I can't directly see other identifying markers for the drive.

Before diving into the sample @Neradoc gave, I'm wondering if the problem I'm trying to solve (getting Mu to work for someone stepping away from default usage like the default device name) is something Mu isn't looking to support.

@LeonHagenaarsKeus
Copy link
Author

Reading boot_out.txt on the drive can be disruptive since it might cause an auto-reload, I don't actually advise doing that. In addition to the other reason mentioned above.

The auto-reload happens after write, not on read. The actual file isn't actually read, only the file's existence is verified. The code.py on the drive will be read though, as that's the one we want to/are expecting to edit. If auto-reload would triggered by reading files, this would probably be a bug in circuitpython/micropython itself.

@Neradoc
Copy link
Contributor

Neradoc commented Jun 27, 2022

The auto-reload happens after write, not on read. The actual file isn't actually read, only the file's existence is verified.

Usually not, but sometimes it does, it depends on the OS I think and other things related to the drive, I disabled reading boot_out.txt by default in discotool (to report CP version) precisely because it caused that behavior, at least on MacOS.

@LeonHagenaarsKeus
Copy link
Author

LeonHagenaarsKeus commented Jun 28, 2022

After looking into the current implementation, which only checks volume names, I feel that using the win32api (currently only used in virtual_environment.py) and wmi (currently not in use in mu), this would probably be a large rewrite, or at least to large for someone just getting to know mu (and the code behind it).

As I'm running a windows machine and have no access to a Mac, I would have to make a lot of assumptions on the Mac and Linux side.

The fix currently in this merge request felt like something I could do quickly (the discussion here took more time then the actual code), this is not the case for the other suggestions. Is there someone I could pair this with? Or is there someone willing to take over? Otherwise I think we should just close this merge request (and possibly the issue that triggered it) with the reasoning why.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rp2040-zero not correctly handled
4 participants