CYW43XXX: Add generic transport layer #14227

pennam · 2021-02-02T15:26:57Z

Summary of changes

Aim of this pr is to add a generic transport layer to COMPONENT_CYW43XXX using unbuffered uart. CYW43XXX_UNBUFFERED_UART directive has been added to keep Cypress hal support available.

40fd126 Adds then CYW43XXX support to PORTENTA_H7 target.

/cc @facchinm

Impact of changes

COMPONENT_CYW43XXX

Migration actions required

Documentation

Pull request type

[] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[X] Feature update (New feature / Functionality change / New API)
[] Major update (Breaking change E.g. Return code change / API behaviour change)

Test results

[] No Tests required for this change (E.g docs only update)
[X] Covered by existing mbed-os tests (Greentea or Unittest)
[] Tests / results supplied as part of this PR

Reviewers

ciarmcom · 2021-02-02T15:30:18Z

@pennam, thank you for your changes.
@ARMmbed/mbed-os-connectivity @ARMmbed/mbed-os-maintainers please review.

0xc0170 · 2021-02-04T20:05:36Z

@ARMmbed/team-cypress please review

kyle-cypress · 2021-02-04T20:47:50Z

connectivity/drivers/ble/FEATURE_BLE/COMPONENT_CYW43XXX/CyH4TransportDriver.cpp

        ++i;
    }
+#if defined(CYW43XXX_UNBUFFERED_UART)
+    while (uart.writeable() == 0);


This doesn't necessarily do the same thing as the HAL counterpart. At least on PSoC devices, uart.writeable() just indicate whether there is space in the serial adapter's FIFO to write a character.

Similarly, putc will block until the character can written to the adapter, but does not wait until it the byte has been sent out over the wire.

I don't know how other MCUs function, but the Mbed HAL documentation for putc doesn't explicitly state that the data will be sent over the wire before it returns:

This is a blocking call, waiting for a peripheral to be available for writing

This is important because deasserting bt_dev_wake before the write to the radio completes can cause communication failures.

The equivalent Mbed HAL-level function to cyhal_uart_is_tx_active would be serial_tx_active, but I don't know of any way in current Mbed to access this function from the UnbufferedSerial class.

Am I correct in supposing that it is important to know that all the bytes have been transmitted before calling deassert_bt_dev_wake ?
@kjbracey-arm Is there anything we can do to know when the hardware queue is empty ?

Correct. Calling serial_tx_active should provide that answer; the question is how to access that when all you have is an UnbufferedSerial object.

serial_tx_active is only available if async is enabled, unfortunate that this is not always enabled and implemented.

I wonder if using the HAL function in this case, here in this component, would cause the trouble (the requirement would be to have async serial available) ?

on our platform uart FIFO is disabled by default,

mbed-os/targets/TARGET_STM/serial_api.c

Line 633 in 77638f9

#if defined(UART_FIFOMODE_DISABLE) // G0/H7/L4/L5/WB

so as far as i can understand uart.writable() and serial_tx_active have the same (opposite) meaning, but this is not true for other platforms using the serial hardware FIFO.

just for the purpose of trying i've made some changes to be able to use serial_tx_active. But something is not working and the application is crashing. I will investigate further on it also running ble integration tests.

just discovered that the problem was me, and serial_tx_active is working as expected, but i cannot found a way to call serial_tx_active hal function directly from CyH4TransportDriver.
Any suggestion to how retrive serial_t *obj from there is really appreciated.
So with 4c7d7e7 i've made small changes to SerialBase and UnbufferedUart to reach serial_tx_active function if DEVICE_SERIAL_ASYNCH is defined.

@0xc0170 @kyle-cypress is 4c7d7e7 an acceptable solution ?

The changes in the BLE HCI driver look reasonable to me.
As do the changes in the SerialBase and UnbufferedSerial, but @0xc0170 may have stronger opinions there than I do.

@0xc0170 @kyle-cypress is 4c7d7e7 an acceptable solution ?

Please send the extension to SerialBase/UnbufferedSerial via new PR (as it's feature update).

pan-

Thanks for the submission, have you run the integration test on the new platform ?

connectivity/drivers/ble/FEATURE_BLE/COMPONENT_CYW43XXX/CyH4TransportDriver.h

pennam · 2021-02-08T08:15:24Z

Thanks for the submission, have you run the integration test on the new platform ?

Hi @pan- if you are referring to mbed-os-bluetooth-integration-testsuite i didn't run them on our platform. I will try to get another hardware to run them and report the results. At the moment we are using BLE functionality through ArduinoCore-mbed and Arduino sketches. In the meanwile i attach mbed test results logs for this branch.

test_report.html.zip (Network tests are failing because no connection was available).

ciarmcom · 2021-02-14T20:50:29Z

This pull request has automatically been marked as stale because it has had no recent activity. @ARMmbed/mbed-os-maintainers, please complete review of the changes to move the PR forward. Thank you for your contributions.

0xc0170 · 2021-02-15T11:06:43Z

just for the purpose of trying i've made some changes to be able to use serial_tx_active. But something is not working and the application is crashing. I will investigate further on it also running ble integration tests.

What's the status of this pull request?

pennam · 2021-02-15T11:40:21Z

@0xc0170 i've just received the hardware to run ble integration tests. If everything works today/tomorrow i will post results.
Regarding serial_tx_active issue i don't have any news. i will restart to work on it as soon as i've finished with the tests.

pennam · 2021-02-15T15:24:34Z

@pan- here the results of ble-integration-tests

====================================================== short test summary info ======================================================
FAILED gap/test_tx_power.py::test_tx_power_effect - assert 0.625 > 15
FAILED gap/test_whitelist_adv_policy.py::test_whitelist_scan_policy[FILTER_ADVERTISING_INCLUDE_UNRESOLVABLE_DIRECTED-empty_whitelist-False]
FAILED gap/test_whitelist_adv_policy.py::test_whitelist_scan_policy[FILTER_ADVERTISING_INCLUDE_UNRESOLVABLE_DIRECTED-random_whitelist-False]
ERROR gap/test_gap_privacy.py::test_resolve_and_filter_unknown_address_with_bond_present[CONNECTABLE_UNDIRECTED] - assert None is ...
ERROR gap/test_gap_privacy.py::test_resolve_and_filter_unknown_address_with_bond_present[SCANNABLE_UNDIRECTED] - assert None is no...
ERROR gap/test_gap_privacy.py::test_resolve_and_filter_unknown_address_with_bond_present[NON_CONNECTABLE_UNDIRECTED] - assert None...
ERROR gap/test_gap_privacy.py::test_non_resolved_address_with_resolve_and_forward[CONNECTABLE_UNDIRECTED] - assert None is not None
ERROR gap/test_gap_privacy.py::test_non_resolved_address_with_resolve_and_forward[SCANNABLE_UNDIRECTED] - assert None is not None
ERROR gap/test_gap_privacy.py::test_non_resolved_address_with_resolve_and_forward[NON_CONNECTABLE_UNDIRECTED] - assert None is not...
ERROR gap/test_gap_privacy.py::test_reject_non_resolved_address_with_one_bond - assert None is not None
ERROR gap/test_gap_state_multiple_connections.py::test_multiple_central_connection - assert None is not None
ERROR gatt_server/test_descriptor_read_write.py::test_clients_read_descriptor - assert None is not None
ERROR gatt_server/test_descriptor_read_write.py::test_clients_write_descriptor - assert None is not None
ERROR gatt_server/test_descriptor_read_write.py::test_cccd_not_shared_between_clients - assert None is not None
=========================== 3 failed, 467 passed, 17 skipped, 2 warnings, 11 errors in 5111.95s (1:25:11) ===========================

ble-integration-tests.log

ciarmcom · 2021-02-27T16:51:02Z

This pull request has automatically been marked as stale because it has had no recent activity. @pennam, please carry out any necessary work to get the changes merged. Thank you for your contributions.

0xc0170

Let's keep this as target update and have new active serial via new pull request (feature update)

pennam · 2021-06-29T11:12:53Z

@ifyall do you have any update on this? Increasing the timeout to 16ms @115200 and to 400us @3000000 should be enough to ensure the 128 byte FIFO worst case. If you agree we can temporary merge this pr with the increased timeout until #14600 gets merged and then substitute the timeout with the sync call.

ifyall · 2021-07-06T19:07:05Z

@pennam, let's merge this. I want my team to run the tests on our hardware, but we won't get to that until next week at the earliest.

Ian

pennam · 2021-07-07T07:31:41Z

I've dropped this commit ae2d79a from this pr cause it was only related to PORTENTA target. I will open another pr that will add BLE support to PORTENTA using this changes.

0xc0170 · 2021-07-08T14:18:48Z

CI started

mbed-ci · 2021-07-08T14:39:05Z

Jenkins CI Test : ✔️ SUCCESS

Build Number: 1 | 🔒 Jenkins CI Job | 🌐 Logs & Artifacts

CLICK for Detailed Summary

jobs	Status
jenkins-ci/mbed-os-ci_unittests	✔️
jenkins-ci/mbed-os-ci_cmake-cloud-example-ARM	✔️
jenkins-ci/mbed-os-ci_cmake-cloud-example-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_build-cloud-example-ARM	✔️
jenkins-ci/mbed-os-ci_build-greentea-ARM	✔️
jenkins-ci/mbed-os-ci_build-cloud-example-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_build-greentea-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_cmake-example-ARM	✔️
jenkins-ci/mbed-os-ci_build-example-ARM	✔️
jenkins-ci/mbed-os-ci_cmake-example-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_build-example-GCC_ARM	✔️
jenkins-ci/mbed-os-ci_greentea-test	✔️

ciarmcom added the release-type: feature label Feb 2, 2021

ciarmcom requested review from a team February 2, 2021 15:30

ciarmcom added the needs: review label Feb 2, 2021

0xc0170 requested a review from a team February 4, 2021 20:05

kyle-cypress reviewed Feb 4, 2021

View reviewed changes

pan- reviewed Feb 5, 2021

View reviewed changes

connectivity/drivers/ble/FEATURE_BLE/COMPONENT_CYW43XXX/CyH4TransportDriver.h Show resolved Hide resolved

ciarmcom added the stale Stale Pull Request label Feb 14, 2021

pennam force-pushed the cyw43xxx_transport_layer branch from 9bec1b8 to 40a3636 Compare February 16, 2021 14:56

mergify bot added needs: work and removed needs: review labels Feb 16, 2021

pennam force-pushed the cyw43xxx_transport_layer branch from 40a3636 to 4c7d7e7 Compare February 16, 2021 15:29

ciarmcom removed the stale Stale Pull Request label Feb 16, 2021

ciarmcom added the stale Stale Pull Request label Feb 27, 2021

0xc0170 requested a review from pan- March 1, 2021 11:41

0xc0170 added needs: review and removed needs: work labels Mar 1, 2021

ciarmcom removed the stale Stale Pull Request label Mar 1, 2021

0xc0170 previously requested changes Mar 11, 2021

View reviewed changes

mergify bot added needs: work and removed needs: review labels Mar 11, 2021

CYW43XXX: Add generic transport layer

c0a8e41

ciarmcom added stale Stale Pull Request and removed stale Stale Pull Request labels Jun 20, 2021

ciarmcom added stale Stale Pull Request and removed stale Stale Pull Request labels Jun 29, 2021

ciarmcom removed the stale Stale Pull Request label Jul 6, 2021

pennam added 2 commits July 7, 2021 09:24

exclude tx and rx if CYW43XXX_UNBUFFERED_UART is defined

c1d33d2

Use fixed wait time

dc4fb8d

pennam force-pushed the cyw43xxx_transport_layer branch 2 times, most recently from caaf54d to dc4fb8d Compare July 7, 2021 07:26

pennam mentioned this pull request Jul 7, 2021

Portenta: Add BLE support using CYW43XXX generic transport layer #14888

Merged

0xc0170 approved these changes Jul 8, 2021

View reviewed changes

mergify bot added needs: CI and removed needs: review labels Jul 8, 2021

0xc0170 added ready for merge and removed needs: CI labels Jul 8, 2021

0xc0170 merged commit 9b0f190 into ARMmbed:master Jul 8, 2021

mergify bot removed the ready for merge label Jul 8, 2021

mbedmain added release-version: 6.13.0 Release-pending and removed release-type: feature Release-pending labels Jul 14, 2021

pennam mentioned this pull request Nov 9, 2021

PORTENTA_H7: CYW4343X add WHD driver arduino/mbed-os#8

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CYW43XXX: Add generic transport layer #14227

CYW43XXX: Add generic transport layer #14227

pennam commented Feb 2, 2021

ciarmcom commented Feb 2, 2021

0xc0170 commented Feb 4, 2021

kyle-cypress Feb 4, 2021

pan- Feb 5, 2021

kyle-cypress Feb 5, 2021

0xc0170 Feb 8, 2021

pennam Feb 8, 2021 •

edited

Loading

pennam Feb 10, 2021

pennam Feb 16, 2021 •

edited

Loading

pennam Mar 1, 2021

kyle-cypress Mar 1, 2021

0xc0170 Mar 11, 2021

pan- left a comment

pennam commented Feb 8, 2021 •

edited

Loading

ciarmcom commented Feb 14, 2021

0xc0170 commented Feb 15, 2021

pennam commented Feb 15, 2021

pennam commented Feb 15, 2021

ciarmcom commented Feb 27, 2021

0xc0170 left a comment

pennam commented Jun 29, 2021

ifyall commented Jul 6, 2021

pennam commented Jul 7, 2021

0xc0170 commented Jul 8, 2021

mbed-ci commented Jul 8, 2021

CYW43XXX: Add generic transport layer #14227

CYW43XXX: Add generic transport layer #14227

Conversation

pennam commented Feb 2, 2021

Summary of changes

Impact of changes

Migration actions required

Documentation

Pull request type

Test results

Reviewers

ciarmcom commented Feb 2, 2021

0xc0170 commented Feb 4, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pennam Feb 8, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pennam Feb 16, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pan- left a comment

Choose a reason for hiding this comment

pennam commented Feb 8, 2021 • edited Loading

ciarmcom commented Feb 14, 2021

0xc0170 commented Feb 15, 2021

pennam commented Feb 15, 2021

pennam commented Feb 15, 2021

ciarmcom commented Feb 27, 2021

0xc0170 left a comment

Choose a reason for hiding this comment

pennam commented Jun 29, 2021

ifyall commented Jul 6, 2021

pennam commented Jul 7, 2021

0xc0170 commented Jul 8, 2021

mbed-ci commented Jul 8, 2021

Jenkins CI Test : ✔️ SUCCESS

Build Number: 1 | 🔒 Jenkins CI Job | 🌐 Logs & Artifacts

pennam Feb 8, 2021 •

edited

Loading

pennam Feb 16, 2021 •

edited

Loading

pennam commented Feb 8, 2021 •

edited

Loading