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

Pr dev/remove hardcoded timeout in cypress bt code #12164

Conversation

shuopeng-deng
Copy link

Summary of changes

Replaced a hardcoded timeout in CyH4TransportDriver.cpp with a cypress hal function. The cypress PUTC hal API only blocks until data has been send into the HW buffer, not until all data has been out of the HW buffer. Modified an API to block until all tx transmit is complete. This allows the removal of a hardcoded timeout in CyH4TransportDriver.cpp that waits for data int the HW buffer to be sent.
The 5000 us delay was insufficient on some Cypress board which may cause the radio to go to sleep prematurely and BLE packets to be delay until the radio wakes up again.

Impact of changes

Migration actions required

Documentation


Pull request type

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

Test results

CYW9P62S1_43438EVB_01_GT.txt

[] 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


Replaced a hardcoded timeout in CyH4TransportDriver.cpp with a cypress
hal function. The cypress PUTC hal API only blocks until data has been
send into the HW buffer, not until all data has been out of the HW
buffer. Modified an API to block untill all tx transmit is complete.
This allows the removal of a hardcoded timeout in
CyH4TransportDriver.cpp that waits for data int the HW buffer to be
sent.
@ciarmcom ciarmcom requested review from maclobdell and a team December 20, 2019 22:00
@ciarmcom
Copy link
Member

@shuopeng-deng, thank you for your changes.
@maclobdell @ARMmbed/mbed-os-pan @ARMmbed/mbed-os-maintainers please review.

Copy link
Member

@bulislaw bulislaw left a comment

Choose a reason for hiding this comment

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

Looks good to me. I'm assuming someone that knows the Cypress code already reviewed it internally?

@adbridge
Copy link
Contributor

@ARMmbed/team-cypress has anybody reviewed this internally from your side ?

@morser499
Copy link

@adbridge yes, this change was reviewed internally.

@0xc0170
Copy link
Contributor

0xc0170 commented Jan 2, 2020

CI started

@mbed-ci
Copy link

mbed-ci commented Jan 2, 2020

Test run: FAILED

Summary: 1 of 11 test jobs failed
Build number : 1
Build artifacts

Failed test jobs:

  • jenkins-ci/mbed-os-ci_cloud-client-pytest

@bulislaw
Copy link
Member

bulislaw commented Jan 2, 2020

Failure doesn't look related, I'll restart the CI.

@mbed-ci
Copy link

mbed-ci commented Jan 2, 2020

Test run: SUCCESS

Summary: 11 of 11 test jobs passed
Build number : 2
Build artifacts

@0xc0170 0xc0170 added release-version: 6.0.0-alpha-1 First pre-release version of 6.0.0 and removed needs: CI labels Jan 2, 2020
@0xc0170 0xc0170 merged commit b8045fb into ARMmbed:master Jan 2, 2020
@sathishm6
Copy link

@adbridge @0xc0170 - This change is required in mbed-os 5.15.1 as well. Can you tag this PR as well for 5.15.1. Thanks.

@adbridge adbridge added Release review required and removed release-version: 6.0.0-alpha-1 First pre-release version of 6.0.0 labels Feb 3, 2020
@mergify
Copy link

mergify bot commented Feb 3, 2020

This PR does not contain release version label after merging.

1 similar comment
@mergify
Copy link

mergify bot commented Feb 3, 2020

This PR does not contain release version label after merging.

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

Successfully merging this pull request may close these issues.

9 participants