ble: update ODIN drivers to v3.5.0 RC1 #8452

aqib-ublox · 2018-10-17T12:10:44Z

Description

This Pull Request contains following feature:

BLE Cordio Stack Ported for ODIN_W2
ODIN drivers are update for BLE cordio

Test Results

The following tests have been performed

mbed-os test for GCC_ARM, ARM and IAR
BLE examples provided by ARM https://github.com/ARMmbed/mbed-os-example-ble

arm_mbed_os_logs.txt
iar_mbed-os_log.txt
gcc_mbed-os_log.txt
iar_ble_logs.txt
arm_ble_logs.txt
gcc_ble_test_logs.txt

Pull request type

[ ] Fix
[ ] Refactor
[x] Target update
[ ] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change

paul-szczepanek-arm · 2018-10-17T13:48:50Z

The failed ble example build error seen in the logs has now been fixed.

We are also currently running the commissioning demo with this new driver.

aqib-ublox · 2018-10-17T13:53:25Z

@paul-szczepanek-arm you are talking about BLE_GAP example right?

cmonr · 2018-10-17T13:53:26Z

@aqib-ublox There are errors in the greentea logs?

cmonr · 2018-10-17T13:54:24Z

@ARMmbed/team-st-mcd Could y'all take a quick look at the target changes?

aqib-ublox · 2018-10-17T13:55:10Z

@cmonr can u point out which logs particularly by name?

paul-szczepanek-arm · 2018-10-17T13:57:13Z

@paul-szczepanek-arm you are talking about BLE_GAP example right?

yes

aqib-ublox · 2018-10-17T14:06:19Z

@paul-szczepanek-arm Yes after your recent commit few minutes ago ARMmbed/mbed-os-example-ble@68afa73 now I am getting BLE_GAP test also building. Thanks @paul-szczepanek-arm

cmonr · 2018-10-17T14:23:58Z

Only took a quick look at the following:

arm_mbed_os_logs.txt
iar_mbed-os_log.txt
gcc_mbed-os_log.txt

Scrolled all the way down, each file had at least one error.

aqib-ublox · 2018-10-17T14:24:44Z

@paul-szczepanek-arm GAP example working fine

arm_ble_GAP_log.txt
gcc_ble_GAP_test_log.txt
iar_ble_GAP_log.txt

this support your verdict. Thanks

The failed ble example build error seen in the logs has now been fixed.

We are also currently running the commissioning demo with this new driver.

aqib-ublox · 2018-10-17T14:35:32Z

@cmonr yeah but these errors is not specific to hardware like threads, mail or hci_transport etc. Am i right?

aqib-ublox · 2018-10-17T16:15:32Z

@cmonr gcc_mbed-os_log.txt updated now all files having single ERROR in hci_test recently commited by ARM against cordio.

cmonr · 2018-10-17T17:30:12Z

@aqib-ublox Thanks for the update.

cmonr · 2018-10-17T17:41:03Z

@aqib-ublox Thanks for the update.

From what I can tell, there are still the following issue with the logs:

All three compilers timeout when testing _mbed-os-features-feature_ble-targets-target_cordio-tests-cordio_hci-transport+
All of the GCC mbed-os-tests-ble_test_case-* tests failed to sync with the target device.
IAR and ARM compilers appear to be having troubles with mbed-os-tests-mbed_hal-sleep and mbed-os-tests-network-wifi tests.
ARM is additionally having issues with mbed-os-tests-mbedmicro-rtos-mbed-heap_and_stack.

Would it be possible to rerun the tests with -vv to get more debug information?

0xc0170 · 2018-10-17T23:24:59Z

features/FEATURE_BLE/targets/TARGET_CORDIO_ODIN_W2/OdinCordioInterface.h

+#define ODIN_CORDIO_INTF_H
+
+#include <stdio.h>
+#include "mbed.h"


Can you include here files that are needed rather than mbed.h ?

0xc0170 · 2018-10-17T23:26:14Z

features/FEATURE_BLE/targets/TARGET_CORDIO_ODIN_W2/HCIDriver.cpp

+
+    /* if event is a command complete event */
+    if (*pMsg == HCI_CMD_CMPL_EVT)
+    {


should this file follow our coding style?

@0xc0170 u r talking about putting baces on same line like:
if( xx ) {

}

I have tried following standard for var and functions naming etc. let me give second look to it .

aqib-ublox · 2018-10-18T06:02:22Z

@cmonr

IAR and ARM compilers appear to be having troubles with mbed-os-tests-mbed_hal-sleep and mbed-os-tests-network-wifi tests.

ARM logs dont havé any issue with wifi test they are passing on ARM toolchain.

arm_mbed-os-wifi_log.txt (Passing)
yes with IAR it's failing even last time(but let me see what we can done with this right now)

Please see log of mbed-os-tests-mbed_hal-sleep test for ARM and IAR toolchain.(they are passing)

arm_mbed_hal-sleep_log.txt (Passing)
iar_mbed_hal-sleep_log.txt (Passing)

All of the GCC mbed-os-tests-ble_test_case-* tests failed to sync with the target device.

mbed-os-tests-ble_test_case* are nothing but ARM BLE examples which are not made on green tea (ht-runner) framework, I added them for myself to generate binary in single compile command, this has nothing to do with mbed provided test sorry for incovenience. (Actually it needs to be ignore Right?)

ARM is additionally having issues with mbed-os-tests-mbedmicro-rtos-mbed-heap_and_stack

they are also passing for ARM toolchain please see log.
arm_mbedmicro-rtos-mbed-heap_and_stack_log.txt

All three compilers timeout when testing _mbed-os-features-feature_ble-targets-target_cordio-tests-cordio_hci-transport+

I have increased this test time but still timeout let me re-evaluate it. (let me run it again with verbose on)

I have run all above test with this branch after recompiling and clearing previous build folder

adustm · 2018-10-18T06:46:34Z

Dear @cmonr

@ARMmbed/team-st-mcd Could y'all take a quick look at the target changes?

I don't see any ST impacted file, this are only ublox and ble files.
Could you remove st team from the review, please ?

Kind regards

aqib-ublox · 2018-10-18T09:08:32Z

@0xc0170 commit compliant with ARM coding standard is committed, I have delpoy changes bare-minimum according to standard adopted in ARM own cordio Class.

aqib-ublox · 2018-10-18T12:11:36Z

@cmonr

All three compilers timeout when testing _mbed-os-features-feature_ble-targets-target_cordio-tests-cordio_hci-transport+
I have increased this test time but still timeout let me re-evaluate it. (let me run it again with verbose on)

Now it start passing just increase a time form 15 to 30 and it is running fine. please see logs
iar_hci_tansport_log.txt
arm_hci_transport_log.txt
gcc_mbed_hci_transport_test_log.txt

cmonr · 2018-10-18T15:30:49Z

@adustm Sure thing.
Was thinking that these modifications qualified as ST changes, but is this not the case because they're module-specific changes?
https://github.com/ARMmbed/mbed-os/pull/8452/files#diff-cad0349ce5f1e4e72982c02516e9b19f
https://github.com/ARMmbed/mbed-os/pull/8452/files#diff-a51b67b98ca84f5e451e6f0ef5a7ba48

Comments addressed.

aqib-ublox · 2018-10-23T12:51:34Z

@0xc0170 ok let me add commit right now and when changes finalized would reset commit history ,rebase and combine all changes with single commit and push, is it fine too?

cmonr · 2018-10-23T17:44:14Z

@paul-szczepanek-arm @0xc0170 Any more thoughts? Would anyone else from @ARMmbed/team-ublox like to review?

paul-szczepanek-arm · 2018-10-24T10:53:27Z

features/FEATURE_BLE/targets/TARGET_CORDIO_ODIN_W2/HCIDriver.cpp

+
+void ble::vendor::odin_w2::HCIDriver::do_terminate()
+{
+    // TODO: ASRI


left over todo, implement or remove and track in jira

that's oudated it is removed in latest commit

actually we have not found any method in Cordio Transport driver which should update HCI baudrate at run time if needed, it assumes baudrate being set at initialization never changes so it was just done in that perspective e.g if we really need to change baudrate first we would terminate current session of HCI and then update baudrate and re-open it but it is not required so I removed TODO in last commit, hope it have clear idea?

paul-szczepanek-arm · 2018-10-24T10:54:23Z

features/FEATURE_BLE/targets/TARGET_CORDIO_ODIN_W2/HCIDriver.cpp

+    shutdown = 0;           // BT Power is OFF
+    wait_ms(20);
+    shutdown = 1;           // BT Power is ON
+    wait_ms(500);


why such a long wait?

This is according to requirement of device power cycle

But i thing BLE power up time is quite satisfactory now, isn't it?

0xc0170 · 2018-10-24T13:28:17Z

Waiting for @ARMmbed/mbed-os-pan approval

paul-szczepanek-arm · 2018-10-24T13:31:05Z

do you need this to go in today or is tomorrow morning OK?

paul-szczepanek-arm · 2018-10-25T08:49:55Z

please ignore, that was a problem with my setup

paul-szczepanek-arm · 2018-10-25T10:54:12Z

Gap::read_phy times out
edit: this is actually fine as it's not supported

asifrizwanubx · 2018-10-25T11:11:05Z

@paul-szczepanek-arm take your time but make sure that we will be able to pass this updated driver in the up-coming release.

paul-szczepanek-arm · 2018-10-25T12:02:27Z

I'm almost done, investigating possible occasional missing scan timeout event. Is this something you've seen @aqib-ublox?

possibly triggered by cancelling scan before timeout by calling stopscan (debugging atm), this is reproducible but right now I cannot tell if it's the driver's fault

aqib-ublox · 2018-10-26T07:13:20Z

@paul-szczepanek-arm no till now i have not investigated this use case please share your finding and case study so that i can check if it is reproducible at my end as well?

paul-szczepanek-arm · 2018-10-26T08:19:14Z

Until I narrow down the problem you can just try to repro it yourself. Start passive scan with some device advertising so that you get some reports but don't let it timeout and cancel it after 200 ms. Repeat a few times and it should fail.

paul-szczepanek-arm · 2018-10-26T09:49:29Z

This is most likely not caused by the driver. I will follow up this issue but I don't think it should stop this merge.

0xc0170 · 2018-10-26T09:56:57Z

/morph build

mbed-ci · 2018-10-26T11:14:38Z

Build : SUCCESS

Build number : 3477
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/8452/

Triggering tests

/morph test
/morph export-build
/morph mbed2-build

mbed-ci · 2018-10-26T19:20:35Z

Test : SUCCESS

Build number : 3266
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/8452/3266

mbed-ci · 2018-10-26T20:13:28Z

Exporter Build : SUCCESS

Build number : 3098
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/exporter/8452/

cmonr · 2018-10-27T14:25:15Z

/morph mbed2-build

BLE support added

2ac6cff

cmonr requested review from a team October 17, 2018 13:53

cmonr added the needs: review label Oct 17, 2018

cmonr requested a review from a team October 17, 2018 13:54

cmonr added needs: work and removed needs: review labels Oct 17, 2018

0xc0170 previously requested changes Oct 17, 2018

View reviewed changes

update with ARM coding standard

c539656

cmonr removed the request for review from a team October 18, 2018 15:31

cmonr added the needs: review label Oct 18, 2018

updating method name to make it more descriptive

9854c1b

cmonr added needs: review and removed needs: work labels Oct 23, 2018

asifrizwanubx approved these changes Oct 24, 2018

View reviewed changes

paul-szczepanek-arm reviewed Oct 24, 2018

View reviewed changes

0xc0170 changed the title ~~Update ODIN drivers to v3.5.0 RC1~~ ble: update ODIN drivers to v3.5.0 RC1 Oct 24, 2018

paul-szczepanek-arm approved these changes Oct 26, 2018

View reviewed changes

0xc0170 approved these changes Oct 26, 2018

View reviewed changes

0xc0170 added needs: CI and removed needs: review labels Oct 26, 2018

cmonr added the release-version: 5.10.3 label Oct 27, 2018

cmonr merged commit 63946d5 into ARMmbed:master Oct 27, 2018

cmonr removed the needs: CI label Oct 27, 2018

ble: update ODIN drivers to v3.5.0 RC1 #8452

ble: update ODIN drivers to v3.5.0 RC1 #8452

Uh oh!

Conversation

aqib-ublox commented Oct 17, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

This Pull Request contains following feature:

Test Results

Pull request type

Uh oh!

paul-szczepanek-arm commented Oct 17, 2018

Uh oh!

aqib-ublox commented Oct 17, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cmonr commented Oct 17, 2018

Uh oh!

cmonr commented Oct 17, 2018

Uh oh!

aqib-ublox commented Oct 17, 2018

Uh oh!

paul-szczepanek-arm commented Oct 17, 2018

Uh oh!

aqib-ublox commented Oct 17, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cmonr commented Oct 17, 2018

Uh oh!

aqib-ublox commented Oct 17, 2018

Uh oh!

aqib-ublox commented Oct 17, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aqib-ublox commented Oct 17, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cmonr commented Oct 17, 2018

Uh oh!

cmonr commented Oct 17, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aqib-ublox Oct 18, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aqib-ublox commented Oct 18, 2018

Uh oh!

adustm commented Oct 18, 2018

Uh oh!

aqib-ublox commented Oct 18, 2018

Uh oh!

aqib-ublox commented Oct 18, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cmonr commented Oct 18, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aqib-ublox commented Oct 23, 2018

Uh oh!

cmonr commented Oct 23, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

0xc0170 commented Oct 24, 2018

Uh oh!

paul-szczepanek-arm commented Oct 24, 2018

aqib-ublox commented Oct 17, 2018 •

edited

Loading

aqib-ublox commented Oct 17, 2018 •

edited

Loading

aqib-ublox commented Oct 17, 2018 •

edited

Loading

aqib-ublox commented Oct 17, 2018 •

edited

Loading

aqib-ublox commented Oct 17, 2018 •

edited

Loading

aqib-ublox Oct 18, 2018 •

edited

Loading

aqib-ublox commented Oct 18, 2018 •

edited

Loading

cmonr commented Oct 18, 2018 •

edited

Loading

paul-szczepanek-arm commented Oct 25, 2018 •

edited

Loading

paul-szczepanek-arm commented Oct 25, 2018 •

edited

Loading

paul-szczepanek-arm commented Oct 25, 2018 •

edited

Loading