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

[WIP] Allow Host Access any Marlin attached SD #19939

Closed

Conversation

rhapsodyv
Copy link
Member

@rhapsodyv rhapsodyv commented Oct 29, 2020

Description

LPC framework have a few requirements for the board work with MSC (exclusive access to SPI 1 and some defaults settings). Not all boards support it. Ex: MKS SGEN L V2.

I sent a PR to LPR Framework to allow the DiskIO functions been overwritten (p3p/pio-framework-arduino-lpc176x#45), so we can use the Marlin Sd2Card API (SPI, SDIO and even flash drive) to share the disc with the host.

This PR does the Marlin part. It will allow the disc being accessed by marlin using the correct board settings. But, it goes beyond: it will allow share with the host any media that marlin can read, even a LCD SD card!

I will take a look if we can do it for others HAL too.

Benefits

Fix #19917
Allow LCD SD to be shared with the host PC

Related Issues

p3p/pio-framework-arduino-lpc176x#45
#19917
#19932

@sjasonsmith
Copy link
Contributor

Does this still require mounting/unmounting to decide whether the device is used over USB or by Marlin?

@rhapsodyv
Copy link
Member Author

Does this still require mounting/unmounting to decide whether the device is used over USB or by Marlin?

No, but yes. As the all writes call being in marlin side, we can put a lock to avoid a parallel write call coming from the USB IRQ.

It will avoid two concurrent writes messing each other. The problem is that a file save call multiples writes. And we can't mix the write calls from different files. So we need a lock in a high layer: the point where the file begin/end to be saved.

But, I didn't look after it yet, if we have this place to put the lock.

@sjasonsmith
Copy link
Contributor

It probably makes sense to just require mount/unmount for now. It would be nice if any solution can (eventually) behave the same with STM32.

@rhapsodyv
Copy link
Member Author

Looks like it's easy to implement msc for stm32duino using marlin sd2card. We just need to implement this callbacks:


USBD_StorageTypeDef USBD_MSC_Template_fops =
{
  STORAGE_Init,
  STORAGE_GetCapacity,
  STORAGE_IsReady,
  STORAGE_IsWriteProtected,
  STORAGE_Read,
  STORAGE_Write,
  STORAGE_GetMaxLun,
  STORAGE_Inquirydata,

};

But we will need to send a PR to stm32duino, like I did with the thumb driver, to setup this api.

@thinkyhead
Copy link
Member

Please enable SD logging with M928 and see if this holds up while that is enabled.

@thinkyhead
Copy link
Member

At the moment we are conflicting with libCMSIS.a because it already contains these new symbols. We must either stop linking with this library, or we must create pass-through substitutes for these functions, under both the LPC1768 and the STM32F1 HALs.

@rhapsodyv
Copy link
Member Author

@thinkyhead I sent a PR to lpc framework here, that will allow overwrite those functions p3p/pio-framework-arduino-lpc176x#45

As soon as it get merged I will update the code in marlin side accordingly.

@moham96
Copy link
Contributor

moham96 commented Nov 6, 2020

@rhapsodyv I tested this fix locally including the lpc framework PR to see if my problem in #19750 is solved.
the host no longer sees the disk as corrupted but I can't mount the disk on the host while it is "attached" in the printer because it only appears to the host as card reader with no medium(no partitions).

[89100.462510] usb 1-1.3: USB disconnect, device number 12
[89192.408194] usb 1-1.3: new full-speed USB device number 13 using dwc_otg
[89192.541885] usb 1-1.3: New USB device found, idVendor=1d50, idProduct=6029, bcdDevice= 1.00
[89192.541909] usb 1-1.3: New USB device strings: Mfr=1, Product=2, SerialNumber=3
[89192.541923] usb 1-1.3: Product: Marlin USB Device
[89192.541936] usb 1-1.3: Manufacturer: marlinfw.org 
[89192.541949] usb 1-1.3: SerialNumber: 0BFF9019AF489C805D3A4A38F50020C7
[89192.543057] cdc_acm 1-1.3:1.0: ttyACM0: USB ACM device
[89192.544299] usb-storage 1-1.3:1.2: USB Mass Storage device detected
[89192.544859] scsi host0: usb-storage 1-1.3:1.2
[89193.601612] scsi 0:0:0:0: Direct-Access     Marlin   SDCard 01        1.0  PQ: 0 ANSI: 0 CCS
[89193.670071] sd 0:0:0:0: [sda] Attached SCSI removable disk

I don't know if this is expected or not, i kinda wished i would be able to use the sdcard on both the host and the printer simultaneously.

Regards

@rhapsodyv
Copy link
Member Author

That's great!! I'm happy that you tested it and solves your issue!

Yes, you can't use the media in the host while it's attached to marlin.

The reason is to avoid SD corruption: we can't do any time of locking, so if the host and the marlin start to write at same time, it will corrupt the SD.

Because of that, only one can access at time. It always worked this way.

We talked a lot for alternatives, but none is safe enough.

Thanks!

@rhapsodyv rhapsodyv added the S: Don't Merge Work in progress or under discussion. label Dec 21, 2020
@rhapsodyv
Copy link
Member Author

I want to work a bit on Multiple Media support, before this get merged.

@moham96
Copy link
Contributor

moham96 commented May 12, 2021

Any idea when will this get merged?

@thinkyhead thinkyhead force-pushed the bugfix-2.0.x branch 2 times, most recently from 27b6361 to 6076e60 Compare May 22, 2021 22:27
thinkyhead added a commit to MarlinFirmware/Configurations that referenced this pull request Feb 5, 2022
@thinkyhead thinkyhead marked this pull request as ready for review February 5, 2022 22:12
@ETE-Design
Copy link
Contributor

@rhapsodyv Will this feature work on an MKS Robin Lite v1.1?

@thisiskeithb
Copy link
Member

Will this feature work on an MKS Robin Lite v1.1?

No. It is only for LPC-based boards.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A: LPC176x PR: Improvement S: Don't Merge Work in progress or under discussion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MKS SGEN_L V2 unable to access SD card from PC via USB
6 participants