Ah, true. You’re correct. Depending on the direction of the flow, disabling OS caching vs. disabling device caching are two different beasts. Disabling OS caching ensures that everything written by the host is written/fetched, but isn’t sufficient to make any caching on the board noticed that it needs to release any buffers it’s holding.
Depending upon which files/directories were held open on the device side, the severity of the impact could vary widely.
Unfortunately, even catching an EVENT_UNPLUGGED may not be sufficient:
Is BusPirate keeping files open for a long time? Perhaps judiciously calling disk_ioctl(pdrv, CTRL_SYNC, nullptr) will get FATFS to retire its blocks, but I don’t see a way without a remount to force it to re-read sectors that might change beneath it.
Gabri and Ian are probably onto the right trail with just forcing it to go completely into park and forcing an unmount/remount to resync on the edges of being exposed to the host. Aggressively closing files might help; I don’t know if fatfs can/will hold onto cached sectors in a file across a close/open cycle. Probably best to just set the focus onto finding a clean point to force unmount cycle…
(I’m about to have this problem in another project, so I’m interested in better understanding it…)
I am talking about a different way. USBMS has a scsi test ready command originating from the host to query about the availability of the storage. If the host gets a negative response it has to show the media as ejected. It keeps polling until the scsi command returns a positive reponse and then media shows inserted.
In the BP5 code base, the following call back does the job:
// Invoked when received Test Unit Ready command.
// return true allowing host to read/write this LUN e.g TF flash card inserted
bool tud_msc_test_unit_ready_cb(uint8_t lun)
I have modified it to latch the eject flag until this command has returned a false, which insures that the host sees the eject event.
The eject event is triggered by calling the following API
tud_msc_start_stop_cb(0, 0, 0, 1);
and insert is triggered by:
tud_msc_start_stop_cb(0, 0, 1, 1);
callers to f_open would need to also call tud_msc_start_stop_cb(0, 0, 0, 1);
and callers to f_calls would also need to call tud_msc_start_stop_cb(0, 0, 1, 1);
Note this solves only one direction (BP5 write a file, host reliably sees it)
To solve both sides of the problem, we need to have a notion of storage ownership.
Initially, a running BP5 that isn’t connected to a user would be in the state where the ownership is host. Writes using USBMS would trigger a file system sync at the completion of each Write10 commands.
When a user connects, Ian is suggesting that the ownership becomes BP5 and the USBMS becomes read only. Writes to the storage would come from the BP5 and synchronization would happen as I described in the preceding reply.
I believe that this simplified synchronization scheme will work and will provide some value. Thanks @ian for suggesting it.
Wow, so that seems to work really well. As I understand it:
When the terminal is not connected, the USBMSC is read/write
When terminal is connected, USBMSC is read only
After any Bus Pirate write, the USBMSC is reset so the PC sees the new files
This all seems to work great. The only issue I had is that it’s slightly disconcerting for the drive window to disappear after write
One small thing I noticed:
Close terminal
write file to USBMSC
Open terminal
ls
New file is not shown until I replug USB. It seems like we should flush the fatfs buffer/remount the storage either:
A. Set a flag on USBMSC updates, remount/refresh if needed on connect
B. Force remount when terminal connects
I have to be honest, I did not realize the “serial port open” function actually worked that reliably, but it seems to.
Another thing is that the refresh after every write should probably go into an underlying storage library function so that it’s transparent to the upper levels (and easy to tweak in a single place).
I didn’t finish the implementation because it does not provide a transparent experience (the drive gets ejected - inserted in the host). I wanted people to get a feel for this new behavior.
Thank you @ian for providing a uf2 to try it out.
I think it’s really nice. In terms of what can be done without going full on MTP, this feels like a super big win.
It may still be possible to allow writes from the PC when the terminal is open if we are aggressive about flagging writes and put some flush/refresh logic in the lower layers.
I’ll wait to hear if we have any more feedback, and then start implementing tomorrow.
@ian I fleshed the code out a little more. I added the code that syncs the file system with the host changes on terminal connection. Note the change in storage_unmount()
yes, I think we can do this. Of course it is going to produce a lot more OS notifications about the drive being ejected since we would have to eject before and insert after evey execution of a command that writes on the storage.
I don’t actually get any notifications on Windows, but if the MSC window is open, it disappears on first read only change, after that it’s quiet in the background.
Updates look nice. I’m sorry, I should have synced with you a bit better. I forgot to push or forgot to mention I had a branch with the msc_disk.h created. Anyhow, there is a new branch called test_file_updates_2 and I pull requested your changes there.
if (tud_cdc_n_connected(0)){
if(!has_been_connected){
has_been_connected = true;
make_usbmsdrive_readonly();
//sync with the host
storage_unmount();
storage_mount();
}
}
else{
if (has_been_connected){
has_been_connected = false;
}
make_usbmsdrive_writable(); //in the if?
}
One little thing stood out, should make_usbmsdrive_writable() be inside the if like this:
if (tud_cdc_n_connected(0)){
if(!has_been_connected){
has_been_connected = true;
make_usbmsdrive_readonly();
//sync with the host
storage_unmount();
storage_mount();
}
}
else{
if (has_been_connected){
has_been_connected = false;
make_usbmsdrive_writable(); //in the if?
}
}
Sure. you can put it under the if. make_usbmsdrive_writable() can be called multiple times because it is a no op if the usb drive is already writable but it is cleaner under the if.
Sorry about this
Is it really a problem ? I would consider it an advantage that we have access layer which can only be used by human and all thumbnails creators and nosy AI powered OS components won’t put/read anything from the device they don’t understand. An API for file access should probably use seperate, dedicated interface like another TTY.
