Skip to content

BLE workflow: readOnly() returns before its own check, so a read-only board is reported as writable #551

Description

@dhalbert

Claude wrote this:

FileTransferClient.readOnly() in js/common/ble-file-transfer.js can never detect anything, because it returns before the code that would do the detecting:

async readOnly() {
    let readonly = false;
    return false;
    // Check if the device is read only
    console.log("Checking if device is read only");
    // Attempt to write a 0-byte temp file and remove it
    const testPath = '/._ble_readonly_check';
    try {
        await this.writeFile(testPath, 0, new Uint8Array(0));
        await this.deleteFile(testPath);
    } catch (e) {
        readonly = true;
    }
    return readonly;
}

Everything after return false; is unreachable. The other transports implement this for real — fsapi-file-transfer.js calls _readOnly() and repl-file-transfer.js returns _isReadOnly — so BLE is the exception rather than this being a convention.

Consequence

The board's filesystem is read-only to CircuitPython whenever USB MSC has it, which is the default for a board plugged into a computer. Two places consume the answer and both get told the drive is writable:

  • checkReadOnly() in js/script.js:408 leaves Save and Save As enabled.
  • FileDialog sets _readOnlyMode from it (js/common/file_dialog.js:128), so the dialog offers operations that cannot succeed.

A save then fails in a way that points at the wrong thing. The device answers STATUS_ERROR_READONLY (0x05), the retry loop in js/script.js runs its attempts, and the user finally gets:

Saving file '…' failed after multiple attempts. Check your connection and try again.

The connection is fine. The drive is mounted over USB. During testing this made repeated save failures look like a bug in the write path, and it took a hex dump of the result to establish that nothing had been written and why.

Possible fixes

The real check is not free — it writes and deletes a temp file on every connect — so short-circuiting it may well have been deliberate, in which case restoring it verbatim is not obviously right.

The cheaper option is to fix the reporting instead. STATUS_ERROR_READONLY already comes back from the device on the first write attempt, so the failure path can name the actual cause without any probing, and the retry loop can stop early rather than retrying something that cannot succeed. @adafruit/ble-file-transfer-js already recognises the status at three call sites.

Either way, the dead code should not stay as it is: a reader has to notice the early return to realise the block below it never runs.

Notes

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions