Skip to content

Fix to allow remount when usb enabled but not msc #2606

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

Merged
merged 1 commit into from
Feb 12, 2020

Conversation

bmeisels
Copy link

@bmeisels bmeisels commented Feb 9, 2020

Addresses #2509.
storage remount now checks if USB MSC is mounted.
Would appreciate more testing before merging.

@dhalbert
Copy link
Collaborator

This looks good to me! @bmeisels one question: Did you test this by powering the board by battery and having USB plugged in, and then disconnecting and attempting the remount()? Assuming you did that, I'm happy to approve.

Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the PR! Unfortunately I think some of the TinyUSB terminology confused things a bit.

Instead of modifying usb.c, please modify usb_msc_flash.c instead. It is already tracking when the host "ejects" the drive. You'll need to set ejected to true on init and then exposed a similar function to check it from storage.

@@ -87,11 +92,13 @@ void usb_background(void) {

// Invoked when device is mounted
void tud_mount_cb(void) {
g_usb_msc_enabled = true;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this needed. mount doesn't mean filesystem mount here unfortunately. It is plugged/unplugged.

@bmeisels
Copy link
Author

@tannewt @dhalbert i have edited the patch according to your suggestion.
The test i performed was the following:

  1. connect initialized (with no code.py) device (with a battery) to Windows PC
  2. connected to repl
  3. called storage.remount("/", False) at this point still fails because the drive is mounted
  4. ejected the drive without unplugging from pc
  5. called storage.remount("/", False) which now works
  6. called storage.remount("/", True)
  7. unplugged and reinserted device
  8. wrote code.py to drive
  9. performed soft reset through terminal which resulted in the code.py running as expected.

I hope this is now correct

@dhalbert dhalbert requested a review from tannewt February 12, 2020 03:59
@tannewt
Copy link
Member

tannewt commented Feb 12, 2020

I'd prefer usb_msc_enabled to be usb_msc_ejected to match the internal name. I can update it tomorrow if that is easiest.

@bmeisels
Copy link
Author

Implemented the requested changes. I did not retest on actual hardware.

Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@tannewt tannewt merged commit 8e8eb07 into adafruit:master Feb 12, 2020
@bmeisels bmeisels deleted the usb-remount-fix branch February 12, 2020 18:40
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.

4 participants