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

Galaxy S9/S7 Verification fail #111

Closed
lukecorbin opened this issue Jun 28, 2018 · 27 comments
Closed

Galaxy S9/S7 Verification fail #111

lukecorbin opened this issue Jun 28, 2018 · 27 comments
Assignees
Milestone

Comments

@lukecorbin
Copy link

lukecorbin commented Jun 28, 2018

I am having success with my Pixel 2/Ipad 2 with the DFU update, but for some reason the Galaxy S9 always fails verification.

The biggest difference between the Pixel 2/Galaxy S9 is the speed. The S9 is updating very fast (~6kbps average).

Is there a way to lower the MTU sizes? Any setting I should try?

06-29 02:45:37.517 16130-17796/com.sigsauer.bdx E/DfuImpl: Executing object failed (error 11) (extended error 12): EXTENDED ERROR
06-29 02:45:37.518 16130-17796/com.sigsauer.bdx E/DfuImpl: Extended Error details: Verification failed

Here are my gradle settings:

compileSdkVersion 27
targetSdkVersion 27

implementation  "com.android.support:support-v4:27.1.1"
implementation  "com.android.support:support-v13:27.1.1"
implementation 'com.android.support:appcompat-v7:27.1.1'
implementation 'no.nordicsemi.android:dfu:1.6.1'

Here is how I start the update:

public void StartNordicDFU(File myFile, String address, String name)
    {

        Log.i(TAG,"StartNordicDFU ");

        final DfuServiceInitiator starter = new DfuServiceInitiator(address)
                .setDeviceName(name)
                .setKeepBond(true);
// If you want to have experimental buttonless DFU feature supported call additionally:
        starter.setUnsafeExperimentalButtonlessServiceInSecureDfuEnabled(true);
// but be aware of this: https://devzone.nordicsemi.com/question/100609/sdk-12-bootloader-erased-after-programming/
// and other issues related to this experimental service.

// Init packet is required by Bootloader/DFU from SDK 7.0+ if HEX or BIN file is given above.
// In case of a ZIP file, the init packet (a DAT file) must be included inside the ZIP file.
        Uri mFileStreamUri = Uri.fromFile(myFile.getAbsoluteFile());

        starter.setZip(mFileStreamUri, myFile.getPath());
        //starter.setForceDfu(true);
       // starter.setDisableNotification(true);
        starter.setForeground(true);


        if ( android.os.Build.VERSION.SDK_INT > 26) {
            starter.createDfuNotificationChannel(this);
        }

        final DfuServiceController controller = starter.start(this, DfuService.class);
// You may use the controller to pause, resume or abort the DFU process.
}
@lukecorbin
Copy link
Author

Here is the logcat:

s9-DfuIpl-Logcat.txt

@lukecorbin
Copy link
Author

lukecorbin commented Jun 29, 2018

The DFU OnError Method is reporting the following:

Error Number = 1036 (0x40C) , Type = 3

@philips77
Copy link
Member

Hi, I'll ask in the sdk team today what could be the reason on this error. If crc values are returned correctly, it's weird that the verification may fail on one phone only.

@lukecorbin
Copy link
Author

Thanks! Any help is appreciated.

After more testing today, It definitely isn't just one phone. We have a Moto E that failed consistently as well.

I tried this same DFU .zip file on the Galaxy S9 with the nRFConnect app and it worked flawlessly. I am hopeful there is some bug in my implementation. However, there just does not seem to be that many knobs in the DfuInitiator for me turn.

@philips77
Copy link
Member

Did you try with nRF Toolbox? There you have the source code as well. In nRF Toolbox I'm not doing anything special, I just start the DFU as normal with the same API :(
The nRF Connect may be a bit slower, as it print logs, but that shouldn't matter much for BLE. Do you have the same settings there as in your app? Check Settings->DFU in nRF Connect.

"Verification failed" error is sent when the signature or the hash doesn't match those in the init file. Are you sure you test the same ZIP on all phones?

have you tried adding some delay before the last Execute? Maybe some time is needed for... something.

@lukecorbin
Copy link
Author

lukecorbin commented Jul 2, 2018

Well, The nrfToolBox fails verification while the nRFConnect does not. So I guess that begs the question, what is different between the two? The DFU files are same since I am selecting them from Google Drive, not uploading them to the respective phones/apps.

I tried adding adding this, but it is never called by the library.

        @Override
        public void onFirmwareValidating(final String deviceAddress)
        {
            controller.pause();
            try{Thread.sleep(3000);}catch(Exception e){}
            controller.resume();
        }

have you tried adding some delay before the last Execute? Maybe some time is needed for... something.

How would i go about do this? Would I need to add the app from source? What file would I need to edit?

I tried adding delays in onProgressChanged, but that didn't seem to help.

            if(percent >= 98)
            {
                controller.pause();
                try{Thread.sleep(3000);}catch(Exception e){}
                controller.resume();
            }

@lukecorbin
Copy link
Author

lukecorbin commented Jul 2, 2018

Update:

I added the library from source and was messing around with the MTU sizes in SecureDfuImpl.java. Our product has the MTU sizes set to 120 in the 14.1 DFU SDK. This has been working on the nRFConnect app and an older IOS pods app for months.

I was able to get success by using the following:

		if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O_MR1 && !android.os.Build.MANUFACTURER.contains("samsung")) {
		//if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP) {
			logi("Requesting MTU = 120");
			requestMtu(120);
			//mService.waitFor(200);
		}

Here is the weird behavior

Galaxy S9 - works when requestMTU is removed, failed when request MTU is enabled.
Pixel 2 - works when requestMTU > 100, fails any lower MTU.
Moto E - works with requestMTU removed but very slowly (.2 kbps).

We updated the IOS pods library and it fails verification with MTU sizes increased as well.

Have you seen such issues when larger MTU sizes were used? It seems like the nRFConnect app is handling them correctly...

@lukecorbin
Copy link
Author

I tried adding delays before the final uploading of the firmware and executing the object. The checksum values are exactly the same and the delays did not seem to affect anything.

Here is the failing version on the S9 with requestMTU having been called:

07-03 07:58:53.874 26510-27927/com.sigsauer.bdx I/DfuImpl: Uploading firmware...
07-03 07:58:53.956 26510-27927/com.sigsauer.bdx I/DfuImpl: Sending Calculate Checksum command (Op Code = 3)
07-03 07:58:54.092 26510-27927/com.sigsauer.bdx I/DfuImpl: Checksum received (Offset = 67808, CRC = FCF5DEB1)
07-03 07:58:54.093 26510-27927/com.sigsauer.bdx I/DfuImpl: Executing data object (Op Code = 4)
07-03 07:58:54.248 26510-27927/com.sigsauer.bdx E/DfuImpl: Executing object failed (error 11) (extended error 12): EXTENDED ERROR
07-03 07:58:54.249 26510-27927/com.sigsauer.bdx E/DfuImpl: Extended Error details: Verification failed

Here is the working version - S9 with requestMTU removed.

07-03 08:03:49.790 30128-31547/com.sigsauer.bdx I/DfuImpl: Luke DELAY!
07-03 08:03:50.792 30128-31547/com.sigsauer.bdx I/DfuImpl: Uploading firmware...
07-03 08:03:50.986 30128-31547/com.sigsauer.bdx I/DfuImpl: Sending Calculate Checksum command (Op Code = 3)
07-03 08:03:51.253 30128-31547/com.sigsauer.bdx I/DfuImpl: Checksum received (Offset = 67808, CRC = FCF5DEB1)
07-03 08:03:51.254 30128-31547/com.sigsauer.bdx I/DfuImpl: Luke DELAY!
07-03 08:03:52.260 30128-31547/com.sigsauer.bdx I/DfuImpl: Executing data object (Op Code = 4)
07-03 08:03:52.428 30128-31547/com.sigsauer.bdx I/DfuImpl: Transfer of 67808 bytes has taken 20411 ms

@philips77
Copy link
Member

As far as I know, the support for higher MTU was added in SDK 15. Did you modify the DFU bootloader on your own? Did you try updating a fw with Samsung S9 running a bootloader from SDK 15?

philips77 added a commit that referenced this issue Jul 3, 2018
The new API allows you to control the MTU from the initiator.
@philips77
Copy link
Member

I've just released version 1.7.0 of the library. In the DfuServiceInitiator you'll now find setMtu(int), which you may use to adjust the MTU.
I know it's not a proper fix, but at least you have the control over the requested MTU.

I still don't know what's the root cause of the problem. As I wrote before, I recommend trying a sample from SDK 15 if:
a) you are using SDK 14.1
b) support for higher MTU has in fact been added in SDK 15 (and this is what I've heard).

@lukecorbin
Copy link
Author

As far as I know, the support for higher MTU was added in SDK 15.
I have seen this language, however I was told by Nordic support to increase the MTU size on my DFU for 14.1. When I upgraded from 12 to 14 I was getting far slower updates until I changed it.

The confusing thing is I have been using an MTU size of 120 for 4 months now with the NRFConnect app and with an older version of the IOS pods library for our own internal setup app. I have had no issues until trying to upgrade to the latest versions (both Android and IOS).

I can't switch to SDK15 since we are launching our product now and need to be able to update existing products with 14.1 w/MTU=120.

As a test, I did try lowering the MTU size on the bootloader firmware to 23 today and everything updated fine. The weird thing is that the update goes much slower when I change the MTU to 23 on the Nordic firmware side verses when I change the MTU to 23 via the setMTU function. I would expect these to be the exact same since the MTU sizes or the same and connection interval values the same. It seems like the library and/or the firmware is doing something different when the MTU sizes are increased.

Is the nRFConnect app using the same version of this library? Should I try an older version?

Version 1.7.0

In order to use this, I have had to use the following switch. The pixel only works with the MTU set, yet all other devices seem to not want the MTU size set. I am very nervous about this. Perhaps there are other phones our there that need special settings as well?

if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O_MR1 && !android.os.Build.MANUFACTURER.contains("samsung") )
        {
            starter.setMtu(120);
            starter.setPacketsReceiptNotificationsValue(10);
        }
        else
        {
            starter.disableMtuRequest();
            starter.setPacketsReceiptNotificationsValue(10);
        }

@philips77
Copy link
Member

Support for higher mtu was added in both libs quite recently. Before they were sending 20 bytes in each packet regardless of mtu.
Still, i think you gave a bug in your modified bootloader as, as you say, behavior is weird and we don't encounter it in sdk 15. I also have no clue where the difference in speed comes from.

Could you ask in devzone? I don't think i can help more here from the lib...
As to the iOS, you'd have to use older version, before mtu was added.

@lukecorbin
Copy link
Author

Well, I tried rolling back to DFU Library Version 1.5.2 and it works on all devices. It just seems there has to be some sort of compatibility issue with the newer versions and the higher MTU sizes on my bootloader, even when I specify a standard MTU size. Theoretically, I think it should not matter what the MTU size is on my firmware if the Android/IOS app only communicates using 20 byte MTU sizes.

@philips77
Copy link
Member

Hi, did you manage to go forward with this issue?

@lukecorbin
Copy link
Author

Well, we went forward with the 1.7.0 approach above and it has been very problematic. We have now found a number of other phones that get the 0x40C verification error. My guess is that some phones need the larger MTU included like the pixel and other don't? It sure seems odd to me that any would need the MTU increased to not get the 0x40C verification error.

My only idea right now is to try rolling back to 1.6.1 and setting notifications lower as this seems to fix some of these issues, but I need to test it on more devices. The problem with this is that it will cripple the MTU increase for newer devices we release on SDK15 in the future....

@philips77
Copy link
Member

Do you get the same issue with the dfu samples from our SDK?
Did you ask this question on Devzone? I guess it's now up to our support team to help you, as may be outside the scope of this library.

@lukecorbin
Copy link
Author

If I change the MTU size on the SDK14 DFU code to 23 verses 120 it works fine with 1.7.0. This isn't a solution though since we already have thousands of units in the field with MTU size of 120.

I only increased it to 120 at the recommendation of a Nordic engineer on a support ticket back in the spring of 2018. We had tested it extensively on Android and IOS with Nordic apps, never with any problems.

It seems to me when MTU size support was added in v1.7.0 it had to have changed something else. I have tried the "starter.disableMtuRequest();" command, but this is not compatible with many phones (Pixel 2, GS8, some Moto's and more). I have tried cloning the 1.7.0 source and removing the MTU request or changing it to different MTU sizes and still no luck with 1.7.0 on all phones.

The frustrating thing is that the whole update completes and then it gives the "Verification Failed" 0x40C message at the very end of the update. So the problem seems to be only at the very last stage of the update when it does final verification.

@philips77
Copy link
Member

Support for higher mtu was added in 1.4.0, in this commit: a41ff75
In 1.7.0 I just added an option to set the mtu value or to disable the request.

The verification at the end is based on the init packet. The CRC of the init packet and firmware are correct, and it seems they are, as the lib send the final execute. I have no idea why would it fail, and depend on mtu, if each time CRC are correct. That's the question for Devzone, I can ask on Monday.

Also, wasn't higher mtu added in dfu bootloader in sdk 15? You added it yourself, based on the recommendation from our engineer, yes?

@philips77
Copy link
Member

I talked with a person responsible for the DFU from the SDK team, and they ask for bootloader logs (if you have them).
The Validation Fails error is thrown only in one place:

if (!fw_hash_ok(p_init, data_addr, data_len))
{
   ret_val = EXT_ERR(NRF_DFU_EXT_ERROR_VERIFICATION_FAILED);
} 

Is it possible for you to break(point) on that line and check the flash contents that is being hashed?

@philips77
Copy link
Member

You could also try the following assert in on_write() in nrf_dfu_ble.c:

ASSERT(p_write_evt->len <= MAX_DFU_PKT_LEN, "Packet received with length longer than allocated buffer."); 

@philips77
Copy link
Member

We found a possible race condition in SDK 14, when the last Execute comes before the last flash write is complete. I'll try to delay the last Execute by few ms and will ping you to give it a spin.

@philips77
Copy link
Member

Hi, could you test the version from bugfix/111 branch? I still have no idea why DFU failed to you on Pixel with MTUs disabled, but perhaps this fixes the Samsung issue? I added 300 ms delay. If 300 is not enough, check more, but it should be OK.

@lukecorbin
Copy link
Author

Aleksander,

Thanks for the help!

bugfix/111 - I added this to my application and saw mixed results.

A couple of times I had this succeed on my GS9, but most of the time it failed. I don't understand why, but it seems related to the update speed which varies each time I run the update.

When I get an update speed ~1.5kb/s, the update succeeds. If the update runs at ~6kb/s then it fails. So it does seem to be tied to the speed of the update for some reason. I don't not understand why I get different speeds on the exact same device with the exact same library. I get the faster update speed on the GS9 almost every time, which causes the failure...

I tried increasing the final delay to 1000/1500ms but saw no change.

I will try to take a look at the DFU logging that you mentioned maybe tonight or tomorrow.

@philips77
Copy link
Member

Did you experiment with PRNs? You have to force-enable them using setPacketsReceiptNotificationsEnabled(boolean) and set the value using setPacketsReceiptNotificationsValue(int).

@lukecorbin
Copy link
Author

I was trying different PRN values, but I had not idea about the force-enable.

I just tried the following and it worked for both GS9 and Pixel 2:

starter.setPacketsReceiptNotificationsEnabled(true);
starter.setPacketsReceiptNotificationsValue(2);

I will test it on more systems to be sure, but it looks like I needed these settings or it won't work. I tried going back to the standard 1.7.0 branch and it worked on that without the delay as well.

@philips77 philips77 self-assigned this Oct 24, 2018
@philips77 philips77 added this to the Version 1.8.0 milestone Oct 24, 2018
@philips77
Copy link
Member

Hi, can we close the issue now?

@Maragues
Copy link

I was experiencing this issue on Huawei phones. Using the default PRN value didn't work, but setting it to 6 fixes it.

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 a pull request may close this issue.

3 participants