BLE FS Using adafruits Simple (not fast) BLE FS Api

[geekbozu]

TODO:

FS Command implementation

• Read
• Read Pacing
• Write
• Write Pacing
• Delete
• Make Dir
• List Dir
• Move File/Folder

Companion/Test App

• Feature Parity To Implementation

Documentation

• Documentation in Infinitime For companion app developers implemented by Add documentation for BLE FS #839

The start of a PR to enabled access to the internal FS over a BLE characteristic.

Discussion on merits of implementation in #750

Currently the marked features are working in this PR,
There is a deviation from the spec which states

Commands always start with a fixed header. The first entry is always the command number itself encoded in a single byte. The number of subsequent entries in the header will vary by command. The entire header must be sent as a unit so set the characteristic with the full header packet. You can combine multiple commands into a single write as long as the complete header is in the packet.

I expect one command per packet with no provisions for more, I could extend this to support more commands per packet...I just do not think the complexity is needed.

I ignore and set all date time information to 0. DO not use date time for anything.

[geekbozu]

This was just a clang format change...There are a few of these littered out ill try to comment on them here

[Avamander]

Meh, keep them. Assuming your config is correct, they have to be merged anyways.

[Avamander]

+1 for the Adafruit protocol 😀 the even slightly bigger ecosystem will help.

[geekbozu]

So as of last night this PR seems to be reading files off the internal flash just fine

However, I'm at an impasse on how I want to handle something. The Spec dictates 2 sets of commands, Read/Write and Read/Write Pacing.
The pacing commands omit the path on data request as a "quicker" / more convenient way of continuing a file transfer thats already been started. EG I need to cache the path variable. I'm not sure which is the best way to handle here. Currently I just have a 256 Byte buffer allocated for the active filePath, which will also have the effect of limiting our path to 256 Bytes, (It's actually less then that due to MTU reasons..its 256 - 16byte ResponseHeadersize - 3byte ATT header. ). Is caching this this way ok...that feels like a lot of memory, the alternative is caching the file handle which is smaller, just over 100bytes I believe, that has the effect of the file staying open between transactions however. Which seems like it could be an easy way to cause issues as the spec has no way to say "Close" a file...

Looking for opinions, Just caching the path (ill optimize it to the actual 238 Bytes it actually can be in the next push..) seems wasteful but I do not see much of another way to handle it.

All of this relies on the fact that I'm expecting all the data in a single BLE packet...currently it does no buffering... rhis is a "deviation" from the spec but, wanted to get it more or less working first :)

In reality this should probably be getting the event doing some basic validity checking and deffering the processing to the SystemTask or an actual FS Tasks, Both of those have large caveats to them in terms of memory usages. Where this is a little smaller with the trade off of every transmission must fit in the MTU size

[lman0]

is this pr will allow to send fonts and picture with gadgetbridge or we need to have someone to make a pull on gadgetbridge for this pull to work?

is this pull will be able to make a secure connection? if not it's worring
Because i don't want that anyone could take everything i put on infinitime without my agreement
or maybe screwing infintime by stuffing the external memory to the max

[Avamander]

@lman0 Neither of those topics are really in the scope of this PR.

[lman0]

thanks , i didnt understood then !

[JF002]

@lman0

is this pr will allow to send fonts and picture with gadgetbridge or we need to have someone to make a pull on gadgetbridge for this pull to work?

This PR is a first step needed to allow companion app to upload assets (bitmaps, fonts, logo,...) into the filesystem (on the external SPI flash memory). When this PR will be merged, companion apps like Gadgetbridge will be able to send files into the watch, but we'll still need to actually use them in InfiniTime (which will probably be done in future PRs). Also, the support for this new protocol will need to be implemented in companion apps.

is this pull will be able to make a secure connection? if not it's worring Because i don't want that anyone could take everything i put on infinitime without my agreement or maybe screwing infintime by stuffing the external memory to the max

BLE security is indeed not in the scope of the PR, but that's definitely something we'll need to implement at some point :)

---

@geekbozu

However, I'm at an impasse on how I want to handle something. The Spec dictates 2 sets of commands, Read/Write and Read/Write Pacing.
The pacing commands omit the path on data request as a "quicker" / more convenient way of continuing a file transfer thats already been started. EG I need to cache the path variable. I'm not sure which is the best way to handle here.

Do we have to implement both sets of commands?

If I understand correctly, "non-pacing" commands allow a stateless implementation : each command contains all the data needed (path, offset,...) to read of write into a file. On the other hand, "pacing" commands allow a more optimized implementation because only the data would be transferred over BLE.

The 'pacing' one look interesting, but maybe we don't need them for a first implementation. We could go for the simpler solution to begin with, and improve it later on. That would reduce the complexity of the integration of this new API and allow us to implement complete functionality (upload/update and use assets in the filesystem, for example) faster.

In reality this should probably be getting the event doing some basic validity checking and deffering the processing to the SystemTask or an actual FS Tasks

I'll have to think a bit more about this one. Inter-task communication bring more complexity (move data efficiently between tasks, task synchronization, data race, timing and timeouts,...) so we must think about it carefully! I think we should try to encapsulate the process needed to read/write a file into a nice class (that would also cache the context, file path,... if needed) so that we can move this object to any task we want later on.

[Avamander]

Do we have to implement both sets of commands?

It would be nice, but not really :D - it's probably our code on both sides for at least some time. It would also be possible to ask Adafruit to make it an optional feature.

[Avamander]

Oh, and we can always take a look at their implementation, if we have any doubts: adafruit/circuitpython#4918

[geekbozu]

So both sets of commands are already implemented. It's more of to implement read/write pacing I need to cache something...making the fs system not stateless. In this implementation I cache the path so I can keep the file closed outside of the routines. That takes a speed hit in exchange for safety.

I can survive with having to have that allocated on the heap. It's just under 256 bytes.

The next question is more of an implementation specific thing. The spec states that you can receive more the one command at a time, and that the data more or less is to be buffered outside of the BLE sub system.

Currently I expect an entire header or header and data to come in a single BLE packet. We have an MTU of 256. So we can receive 253bytes at once after the ATT header. This deviates from the listed spec. We can do this since we have a large MTU size. The alternative involves passing All the data to a queue to the system task and then processing it there. It's doable, and freertos has the mechanisms designed for just this. It's just going to be more expensive then the current implementation so I am looking for guidance on what we want.

[Avamander]

@geekbozu is there a blocker to this PR? Could external FS access land in next InfiniTime?

[geekbozu]

It heavily depends on my IRL schedule. There is quite a bit Todo I hope to have a task lost up today, and to continue chipping away at it!
There are no major blockers as of the moment implementation wise. Just time to work on it.

[geekbozu]

Added a task list for this, It is slowly nearing completion!
I added some comments in the OP about deviations from the OG spec as well as asked questions about it above.
Feedback is requested!

[geekbozu]

Move works! At this point its feature complete. It needs some better status code returns as well as some documentation.
Have we decided where that documentation is going to live yet?