bentu-noodoe commented 1 year ago

Hello,

There''s an option pipelineDepth in FirmwareUpgradeConfiguration, and with pipelineDepth set to larger than 1 can increase the transfer speed when using ImageManager to upload firmware files. According to the documents, I suppose the feature is called SMP Pipeline.

My question is,

Is it possible to supoort such feature in FileSystemManager?

Unlike ImageManager, FileSystemManager's upload method doesn't take FirmwareUpgradeConfiguration, thus it doens't benefit from SMP Pipeline. We found that the android version of the library did supoort the feature in both of their ImageManager and FileSystemManager, just wondering if there're any concerns so it's not implemented on iOS.

Thanks in advance, really appreciate all the hard works from contributors.

Btw, We're uploading files ~500kB to our devices, and we found that the upload speed on iOS was aorund 2kB/s. It'd take longer than 4mins to upload a single file, and we've got 4+ files to transfer, so we're looking for solutions to boost up the speed.

dinesharjani commented 12 months ago

Hey @bentu-noodoe - basically I was not aware of this. I was handed the 'keys to this kingdom' by being told 'there's a small feature we need help with called pipelining. You can do that easily' and, I took it and well as you can see from this codebase, it's a very deep rabbit hole.

This sounds like something that should be implemented. I'm going to start looking into it today I think.

bentu-noodoe commented 11 months ago

Good to hear that, it'd save us lots of effort, thank you so much!

dinesharjani commented 11 months ago

@bentu-noodoe so I think this will be for next version. I'm preparing a bugfix release, and then I'll follow-up with this work and some other as part of 1.4, since I think I'll make some breaking changes to the API.

dinesharjani commented 11 months ago

@bentu-noodoe https://github.com/NordicSemiconductor/IOS-nRF-Connect-Device-Manager/pull/131

bentu-noodoe commented 10 months ago

Hello @dinesharjani

I've tested the new releases both 1.4.0 and 1.4.1, finding that the upload speed was still around 2kB/s, just like it was before 1.4.0.

Here's how I created my configuration:

var config = FirmwareUpgradeConfiguration()
config.pipelineDepth = 3
config.byteAlignment = .fourByte

I used the same config with ImageManager, and it was much faster, so I guess the config was ok.

Digged a little bit into the project, it seemed that FileSystemManager didn't make uses of pipelineDepth at all.

On the other hand, ImageManager used it in its uploadCallback to..., I don't know, not familiar to the project 😂. Well I think, in the end, it added many operations to the operationQueue altogether, which FileSystemManager didn't. Just a guess, ignore me if I was wrong.

Could you check if it's actually an issue, or I just didn't do it the right way?

dinesharjani commented 10 months ago

I just run a test and I got better upload speed on nRF52840. It's not as fast as DFU, I got about 6-7 kbps. But it is an improvement. Are you using FileSystemManager's upload() that takes in the FirmwareUpgradeConfiguration you showed? Those parameters look okay.

bentu-noodoe commented 10 months ago

I just run a test and I got better upload speed on nRF52840. It's not as fast as DFU, I got about 6-7 kbps. But it is an improvement. Are you using FileSystemManager's upload() that takes in the FirmwareUpgradeConfiguration you showed? Those parameters look okay.

Yes I used the config in upload().

My codes look like

        let transport = McuMgrBleTransport(peripheral)
        var config = FirmwareUpgradeConfiguration()
        config.pipelineDepth = 3
        config.byteAlignment = .fourByte
        switch someType {
        case .typesUsingImageManager:
            imageManager = ImageManager(transporter: transport)
            if !imageManager!.upload(images: images, using: config, delegate: self) {
                handleFailure()
            }
            break
        case .typesUsingFileSystemManager:
            fsManager = FileSystemManager(transporter: transport)
            if !fsManager!.upload(name: name, data: data, using: config, delegate: self) {
                handleFailure()
            }
            break
        }

I just tested it, and ImageManager is actually much faster than FileSystemManager.

To provide more information, our project uses nRF52840 for our hardware. We also have our android app, in which we use the same settings for pipelineDepth and byteAlignment(the naming may be different), and the transfer speed for FileSystemManager is like around 6kB/s .

I turned to my colleague, who works on the firmware team, and he asked if you could share your settings of BLE in your firmware? Maybe we actually missed something here.

Thank you for your time!

dinesharjani commented 10 months ago

So this is what my overlay-fs.conf looks like:

`

Enable the LittleFS file system.

CONFIG_FLASH_MAP=y CONFIG_FILE_SYSTEM=y CONFIG_FILE_SYSTEM_LITTLEFS=y

Add 256 bytes to accommodate upload command (lfs_stat overflows)

CONFIG_SYSTEM_WORKQUEUE_STACK_SIZE=2304

Enable file system commands

CONFIG_MCUMGR_GRP_FS=y `

This is, from a fresh smp_svr that has the bluetooth overlay as well enabled. On SDK & toolchain version 2.4.2. Another option could be, I run the code again, and send you the hex to flash it on nrf52840.

Also, what file sizes to upload are you testing with? If the file is too small, then it might not build up enough speed. But if you make it too big, the upload will fail because this feature doesn't like big files from my experience. It's tricky, I know :)

bentu-noodoe commented 10 months ago

The differeces that I found between the .conf files are,

Our CONFIG_SYSTEM_WORKQUEUE_STACK_SIZE was set to 4096.
We had our SDK and toolchain version on 2.0.2, and it seemed that CONFIG_MCUMGR_GRP_FS was not supported on that version.

Our full .conf prj.conf.zip .

Could you share you full .conf file so we could have more detail?

Depending on the purpose, the size of files to upload are from 40KB to 400 KB each.

Another option could be, I run the code again, and send you the hex to flash it on nrf52840.

Yes, please. I think it would be a good way to figure out which end the problem is on.

Pardon me for asking so much, I really appreciate the help.

dinesharjani commented 10 months ago

SMP_SVR [ConfFiles.zip](https://github.com/NordicSemiconductor/IOS-nRF-Connect-Device-Manager/files/13266185/ConfFiles.zip) _FS_merged.zip

So the .conf files are now split in 'overlays'. There's an overlay for Bluetooth and another one for FileSystem. I've attached a ZIP with the prj.conf plus the two overlay files. And I've attached a working .hex for nRF52840 that I use for test. It does 6-7kbps with 4KB files for example.

And no, you're not asking too much :)

bentu-noodoe commented 9 months ago

I flashed the .hex to our nRF52840, and I ended up writing a test project, to test with different file sizes, each with/without SMP Pipeline.

For 4 scenarios: 4KB file * 3 without SMP Pipeline, 4KB file * 3 with SMP Pipeline, Single 300KB file without SMP Pipeline, Single 300KB file with SMP Pipeline, and I tested 3 times for each and took average for those results.

4KB file * 3 without SMP Pipeline: 4198 kB/s
4KB file * 3 with SMP Pipeline: 4325 kB/s ^ Multiple small files
Single 300KB file without SMP Pipeline: 4822 kB/s (4720, 4858, 4888)
Single 300KB file with SMP Pipeline: 4668 kB/s (4803, 4581, 4622) *Yes it's slower ^ Single/Multiple large files

Things that I noticed:

The speed doesn't seem to increace very much
It never gets to send more than ~16kB of data, it usually fails around there, and sometimes it fails earlier. And if it fails, it usually just fails at the beginning the next time I upload, I had to erase and flash the board again to make it work.

So my question would be:

Should I expect the speed to be faster in those cases that enables SMP Pipeline? I understand our own project transfers much slower, but I think if I could expect, let's say 100%, faster with it enabled in the test project, I could as well expect the same improvement in our own project, like 4->8, or 2->4.
Is it a known issue for zephyr-rtos that fs manager cannot transfer files whose size is larger than a certain value? As you mentioned, "...if you make it too big, the upload will fail because this feature doesn't like big files...", but I couldn't find related issues in this repo or in zephyr-rtos. (I just knew that our firmware team did some tricks so we're able to transfer larger files, so it looks like a bug or something to me)

dinesharjani commented 9 months ago

It is faster. If I run nRF Connect Device Manager Example app (which is in this GitHub project) and modify the start() function like so:

var configuration = FirmwareUpgradeConfiguration(); configuration.pipelineDepth = 4; configuration.reassemblyBufferSize = 2475; _ = fsManager.upload(name: destination.text!, data: fileData!, using: configuration, delegate: self)

(GitHub is pretty bad at formatting code, hence why I added semicolons)

I see 8-10 kbps speed on my iPhone XR, compared to 4-5 kbps without. I wrote a change that shows in the 'Uploading...' text the speed. I'll make the PR and push it to master.

This is using the .hex file I sent you the last time.

As for your size question, yes, your 4KB ballpark seems accurate. I don't have much insight into this, since I'm relatively 'new' to this side of the library. The most important thing for us is 'DFU, DFU & DFU', so I put my focus there. But you asked for this to be added and I thought it would be good to add this functionality as well to make the library as complete as possible. When I asked about the size limit, I got directed to the littlefs GitHub page, which I haven't had the time to look into (every day, there's something new to fix). But if you guys managed to improve the situation, I'm very interested in knowing how :)

Let me know if there's anything more I can do.

NordicSemiconductor / IOS-nRF-Connect-Device-Manager

Support SMP Pipeline in FileSystemManager #122

Enable the LittleFS file system.

Add 256 bytes to accommodate upload command (lfs_stat overflows)

Enable file system commands