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

samples: peripheral_uart: Fix hard-coded timeout value #12454

Closed
wants to merge 1 commit into from

Conversation

pideu-mh
Copy link

Calls to uart_rx_enable() require a timeout argument, for which UART_WAIT_FOR_RX is used in 2 of 3 calls in this application.

The initial init call, however, uses a hardcoded value of 50 (milliseconds) instead of UART_WAIT_FOR_RX which is clearly an oversight.

@CLAassistant
Copy link

CLAassistant commented Sep 27, 2023

CLA assistant check
All committers have signed the CLA.

@github-actions github-actions bot added the changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. label Sep 27, 2023
@NordicBuilder
Copy link
Contributor

Thank you for your contribution!
It seems you are not a member of the nrfconnect GitHub organization. External contributions are handled as follows:
Large contributions, affecting multiple subsystems for example, may be rejected if they are complex, may introduce regressions due to lack of test coverage, or if they are not consistent with the architecture of nRF Connect SDK.
PRs will be run in our continuous integration (CI) test system.
If CI passes, PRs will be tagged for review and merged on successful completion of review. You may be asked to make some modifications to your contribution during review.
If CI fails, PRs may be rejected or may be tagged for review and rework.
PRs that become outdated due to other changes in the repository may be rejected or rework requested.
External contributions will be prioritized for review based on the relevance to current development efforts in nRF Connect SDK. Bug fix PRs will be prioritized.
You may raise issues or ask for help from our Technical Support team by visiting https://devzone.nordicsemi.com/.

Note: This comment is automatically posted and updated by the Contribs GitHub Action.

@NordicBuilder NordicBuilder added the external External contribution label Sep 27, 2023
@KAGA164
Copy link
Contributor

KAGA164 commented Oct 3, 2023

Thanks for your contribution @pideu-mh. I check the reason why CI failed:

6: UC4 Commit message body line exceeds max length (75>72): "The initial init call, however, uses a hardcoded value of 50 (milliseconds)"
Error: Process completed with exit code 1.

Could you format your commit message according to above requirement?

Calls to uart_rx_enable() require a timeout argument, for which
UART_WAIT_FOR_RX is used in 2 of 3 calls in this application.

The initial init call, however, uses a hardcoded value of 50
(milliseconds) instead of UART_WAIT_FOR_RX which is clearly an
oversight.

Signed-off-by: Michael Hunold <[email protected]>
@pideu-mh
Copy link
Author

pideu-mh commented Oct 4, 2023

Could you format your commit message according to above requirement?

I'm sorry for the additional work on your side. I have now reformatted the commit message and force pushed an update.

@miha-nordic miha-nordic added CI-Requested Approves single commit for CI tests on Internal HW CI-trusted-author The author of the PR is trusted. All future commits will be tested in PR. labels Oct 5, 2023
@NordicBuilder
Copy link
Contributor

Test specification

CI/Jenkins/NRF

  • Integration Platforms

CI/Jenkins/integration

Test Module File based changes Manually selected West overwrite
test-fw-nrfconnect-ble X
test-fw-nrfconnect-ble_samples X

Detailed information of selected test modules

Note: This message is automatically posted and updated by the CI

@KAGA164
Copy link
Contributor

KAGA164 commented Oct 5, 2023

One more thing. The PR target is wrong. This should go to ncs main branch. @pideu-mh Could you change PR target?

@carlescufi
Copy link
Contributor

@pideu-mh can you please retarget this PR against the main branch and then we'll take care of backporting to v2.4?

Copy link

github-actions bot commented Dec 5, 2023

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

@github-actions github-actions bot added the Stale label Dec 5, 2023
@github-actions github-actions bot closed this Dec 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog-entry-required Update changelog before merge. Remove label if entry is not needed or already added. CI-Requested Approves single commit for CI tests on Internal HW CI-trusted-author The author of the PR is trusted. All future commits will be tested in PR. external External contribution Stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants