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

Set the default max MTU size within the DFU transport to 23 #293

Merged
merged 2 commits into from Feb 27, 2023

Conversation

Montvydas
Copy link
Contributor

Changed the default max MTU size within the DFU transport to 23, which is the required size for the legacy bootloader to perform a successful OTA with the latest Nordic-DFU-Library. This resolves #287.

…h is the required size for the legacy bootloader to perform a successful OTA with the latest Nordic-DFU-Library.
Copy link
Member

@hathach hathach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, it seems to be the lack of memory issue with higher MTU. Though 23 is a bit slow, can you try with a few higher MTU such as 50, 70, or 100 to see if we could have an optimum MTU.

@Montvydas
Copy link
Contributor Author

So I think it is a bit different than that. As I mentioned on the issue, one can use an older version of nRF Connect App e.g. v4.24.3 and then the MTU can be left as 247 (or to be honest, any number). You see, I tried experimenting with different MTU sizes and nRF Connect versions and noticed this:

  • With nRF Connect v4.24.3 I can set MTU to 15, 23, 50, 247 and it will work, although the average speed will always be 4 kB/s. Meaning, the MTU value within the Bootloader isn't used at all, it is set by the nRF Connect App.
  • With nRF Connect v4.26.0 (latest) the upload speed will be different depending on the MTU value. With MTU=15 it will not even start uploading, MTU=23 the avg speed=2.8 kB/s, with MTU=50 the avg. speed=5.7 kB/s, with MTU=247 the avg. speed=~70 kB/s (Using this the update rate is choppy, so at first it updates 60% straight away, then another 20%, then 10%..). However only the MTU=23 will complete the firmware upload successfully, while the others will hang up after reaching 100% (or will not even start at all) and will never be able to boot into the application mode.

I searched this up within Nordic-DFU-Library and I found some proof that the current Nordic DFU Library allows using higher MTU values than 23, but 'MTU > 23 is only supported in the official Secure DFU from SDK 15 onwards.', more on the matter here and here. Knowing that the Adafruit nRF Bootloader is an older unsecure version we can safely assume that for this to work we have to set the MTU to 23. The only thing I am not fully sure is why when I set the MTU to 23 and use nRF Connect v4.26.0, the avg. firmware upload speed is 2.8 kB/s, which is lower than 4 kB/s, which can be achieved with nRF Connect v4.24.3. The only explanation is that something else changed within Nordic DFU Library, that affects this, which has nothing to do with the MTU size.

Note this does not affect the MTU speed when used within the application mode, I tested this using the throughput.ino example by requesting to change to different MTU values there:

MTU=23
Sending 20000 bytes ...
Sent 20000 bytes in 1.15 seconds.
Speed : 17.33 KB/s.
---
MTU=247
Sending 244000 bytes ...
Sent 244000 bytes in 3.34 seconds.
Speed : 73.05 KB/s.

Regardless, now we know that this fix will not affect the older version of the Nordic-DFU-Library (e.g. the one used with nRF Connect App v4.24.3), since it would enforces the MTU to be 23 regardless (meaning it is backwards compatible). Then, it will ensure that we can use Adafruit nRF52 Bootloader with the latest version of the nRF Connect App, nRF Toolbox and the Nordic-DFU-Library, meaning it will work with Android 12 and will also support Flutter!

Copy link
Member

@hathach hathach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you very much for your sufficient testing. Indeed this cross-version compatible is a bit headache, which explain why I haven't experienced the issue under my mobile setup and others has quite a bit of issue with this. Changing back to normal MTU=23 even though is a bit slow but seems to be the safest thing to do for now. When having more time to test with nrf mobile API, we will re-increase it if needded.

@hathach hathach merged commit 80c3a84 into adafruit:master Feb 27, 2023
@Montvydas
Copy link
Contributor Author

thank you very much for your sufficient testing. Indeed this cross-version compatible is a bit headache, which explain why I haven't experienced the issue under my mobile setup and others has quite a bit of issue with this. Changing back to normal MTU=23 even though is a bit slow but seems to be the safest thing to do for now. When having more time to test with nrf mobile API, we will re-increase it if needded.

Just to clarify, I went through the comments and discussions on the Nordic DFU Library github. I found out that the previous version of Nordic DFU Library was forcing MTU=23 regardless of the value set within the bootloader. This was due to both the Nordic DFU Library not being able to support higher MTUs nor the non-secure bootloader. Then, in the latest versions of the Nordic DFU Library they enabled support for higher values of MTUs, which work with a configured secure bootloader. So keeping this in mind, if we want higher MTUs in the future, the bootloader will have to become secure, however I understand that would require a whole rewriting of the bootloader, is that right?

@rcarteraz
Copy link

Hmm. Has anyone else noticed this? Making this change seems to resolve the issue with Android devices and OTAs hanging at 100%. I can now successfully flash them. However, it appears to cause issues with Apple devices now. Previously, I had no issues flashing, in fact they performed much better with Apple. But, now I'm unable to flash with my iPhone/Macbook even though my iPad (5th gen) works. Of course Android is working now too.

@Montvydas
Copy link
Contributor Author

@rcarteraz Could you please share a bit more information on the versions that you have tested and what did you try e.g. can you fill this in:

  • With nRF Connect v4.24.3, without this patch works / doesn't work on iPhone/Macbook.
  • With nRF Connect v4.24.3, with this patch works / doesn't work on iPhone/Macbook.
  • With nRF Connect v4.26.0 , without this patch works / doesn't work on iPhone/Macbook.
  • With nRF Connect v4.26.0 , with this patch works / doesn't work on iPhone/Macbook.

I am not sure if you can install an older version of nRF Connect on iPhone, but maybe on Macbook it should be possible finding an older version. When you do this, I could then take a look and see if there are any obvious fixes.

What I am trying to understand here is whether this setting breaks something that worked or instead it fixes the issue for the Android system, but does not fully fix for Apple Ecosystem.

@rcarteraz
Copy link

@rcarteraz Could you please share a bit more information on the versions that you have tested and what did you try e.g. can you fill this in:

  • With nRF Connect v4.24.3, without this patch works / doesn't work on iPhone/Macbook.
  • With nRF Connect v4.24.3, with this patch works / doesn't work on iPhone/Macbook.
  • With nRF Connect v4.26.0 , without this patch works / doesn't work on iPhone/Macbook.
  • With nRF Connect v4.26.0 , with this patch works / doesn't work on iPhone/Macbook.

I am not sure if you can install an older version of nRF Connect on iPhone, but maybe on Macbook it should be possible finding an older version. When you do this, I could then take a look and see if there are any obvious fixes.

What I am trying to understand here is whether this setting breaks something that worked or instead it fixes the issue for the Android system, but does not fully fix for Apple Ecosystem.

Yeah I can take a stab at trying that tomorrow (rather later today). I did find that for iOS/macOS if I enabled PRN and changed the packets to 6 in the settings it would successfully flash. Weirdly, that's not required on my iPad though it just works still. This was with the DFU app, not nRF connect but I think that can be set there too and I'll try.

I'll add more later.

@Montvydas
Copy link
Contributor Author

@rcarteraz So the DFU app uses the same library, so ultimately it's the same thing. I am simply not sure which version of DFU app did the updates the "old" way... I am sure it's possible to track this down from the github release list on nRF repo.

@rcarteraz
Copy link

@rcarteraz Could you please share a bit more information on the versions that you have tested and what did you try e.g. can you fill this in:

  • With nRF Connect v4.24.3, without this patch works / doesn't work on iPhone/Macbook.
  • With nRF Connect v4.24.3, with this patch works / doesn't work on iPhone/Macbook.
  • With nRF Connect v4.26.0 , without this patch works / doesn't work on iPhone/Macbook.
  • With nRF Connect v4.26.0 , with this patch works / doesn't work on iPhone/Macbook.

I am not sure if you can install an older version of nRF Connect on iPhone, but maybe on Macbook it should be possible finding an older version. When you do this, I could then take a look and see if there are any obvious fixes.

What I am trying to understand here is whether this setting breaks something that worked or instead it fixes the issue for the Android system, but does not fully fix for Apple Ecosystem.

It appears the apple apps do not use those version numbers. For nRF Connect the latest version is 2.6.4 and the DFU app is 1.1. Additionally, I'm not aware of a way to test the older versions as the app store does not allow you to select the version.

@rcarteraz
Copy link

@rcarteraz So the DFU app uses the same library, so ultimately it's the same thing. I am simply not sure which version of DFU app did the updates the "old" way... I am sure it's possible to track this down from the github release list on nRF repo.

The interesting thing though, on iOS, there wasn't an issue with OTA updates like there was on Android. It worked quite well on iOS. Would upload at 11kbs and finish every time. Until, testing this change to fix Android.

I've found a way to make it work on Apple but it's not ideal and also really slow, around 3.8kbs. But if you go into the settings of the DFU app and enable PRN (Packets Receipt Notification) and change the value to 6. It'll work. I tried 12 but it didn't work so not sure if there's more options between those two number.
Screenshot 2023-05-20 at 10 55 11 AM

Will see if there's an option like this in nRF Connect.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DFU OTA with NRF Connect stuck at 100%
3 participants