Skip to content

Cellular: Add AT handler buffer size to configuration #14716

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jun 29, 2021
Merged

Cellular: Add AT handler buffer size to configuration #14716

merged 1 commit into from
Jun 29, 2021

Conversation

boraozgen
Copy link
Contributor

@boraozgen boraozgen commented May 31, 2021 •
edited
Loading

Summary of changes

Some modem-specific configuration commands require a buffer larger than 32 bytes. This PR moves the buffer size definition to the configuration so that it is configurable by the user.

I believe this is a patch update since the default behavior does not change but please correct me if I'm wrong.

Impact of changes

Migration actions required

Documentation

None

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 

[] 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 ciarmcom added the release-type: patch Indentifies a PR as containing just a patch label May 31, 2021
@ciarmcom ciarmcom requested a review from a team May 31, 2021 13:00
@ciarmcom
Copy link
Member

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

0xc0170
0xc0170 previously requested changes Jun 2, 2021
View reviewed changes
Copy link
Contributor

@0xc0170 0xc0170 left a comment

Choose a reason for hiding this comment

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

The change requires unittests update (see failures in Travis)

@boraozgen
Copy link
Contributor Author

I don't get this. Where are the configuration variables retrieved from in unit tests? MBED_CONF_CELLULAR_AT_HANDLER_BUFFER_SIZE seems to be not found but it should have been generated by the added variable in mbed-lib.json.

@boraozgen
Copy link
Contributor Author

I guess I found it: unittest-test-flags must be modified. On it.

@boraozgen boraozgen force-pushed the at-handler-buffer-size-config branch from 4cf4369 to be374fb Compare June 2, 2021 14:25
@mergify mergify bot dismissed 0xc0170’s stale review June 2, 2021 14:25

Pull request has been modified.

@mergify
Copy link

mergify bot commented Jun 2, 2021

This PR cannot be merged due to conflicts. Please rebase to resolve them.

@boraozgen boraozgen force-pushed the at-handler-buffer-size-config branch from be374fb to 3ad65df Compare June 2, 2021 14:34
@boraozgen
Copy link
Contributor Author

Can I run the tests on my host? It seems to need some trial and error to find out which ones need the macro.

@0xc0170
Copy link
Contributor

0xc0170 commented Jun 7, 2021

@boraozgen Yes, follow https://os.mbed.com/docs/mbed-os/latest/debug-test/unit-testing.html (section how to run the test is lower)

@boraozgen boraozgen force-pushed the at-handler-buffer-size-config branch from 3ad65df to 8c1750a Compare June 8, 2021 09:52
@boraozgen
Copy link
Contributor Author

I had multiple issues building the unit tests on a Windows machine, gave up in the end. Gonna use the CI, shouldn't take too many tries.
Why is it failing at "license check" now?

@0xc0170
Copy link
Contributor

0xc0170 commented Jun 8, 2021

Either dep broke in the python packages or Travis hiccup. I restarted it. I'll create one PR soon , will test if there is deps problem.

@0xc0170
Copy link
Contributor

0xc0170 commented Jun 8, 2021

I can't even restart Travis. Can you just ammend a commit and push force again to restart the travis? Subsequent PRs did not fail as I reviewed them just now. The failure is related to environment in Travis so restart should clear it.

@0xc0170
Copy link
Contributor

0xc0170 commented Jun 8, 2021

Wait, https://status.python.org/ - outage . lets wait until it settles.

@0xc0170 0xc0170 closed this Jun 9, 2021
@0xc0170
Copy link
Contributor

0xc0170 commented Jun 9, 2021

Travis cant be restarted, I'll reopen in a minute to trigger a new Travis build

@mergify mergify bot removed component: connectivity needs: work release-type: patch Indentifies a PR as containing just a patch labels Jun 9, 2021
@0xc0170 0xc0170 reopened this Jun 9, 2021
@0xc0170
Copy link
Contributor

0xc0170 commented Jun 9, 2021

I found this in travis log:

Could not authorize build request for ARMmbed/mbed-os

It did not help. @boraozgen can you amend the last commit without any change and push force to restart Travis

@0xc0170 0xc0170 requested a review from a team June 11, 2021 12:10
@0xc0170
Copy link
Contributor

0xc0170 commented Jun 11, 2021

Unittests failed again (in jenkins, lets wait for Travis)

@mbed-ci
Copy link

mbed-ci commented Jun 11, 2021

Jenkins CI Test : ❌ FAILED

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_build-cloud-example-ARM ✔️
jenkins-ci/mbed-os-ci_build-cloud-example-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_cmake-cloud-example-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_cmake-example-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_build-greentea-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_build-example-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_build-example-ARM ✔️
jenkins-ci/mbed-os-ci_build-greentea-ARM ✔️
jenkins-ci/mbed-os-ci_cmake-example-ARM ✔️

@mergify mergify bot added needs: work and removed needs: CI labels Jun 11, 2021
@boraozgen
Copy link
Contributor Author

I'll try to run the tests on a linux machine...

@boraozgen boraozgen force-pushed the at-handler-buffer-size-config branch from 9795b39 to 8705a6a Compare June 15, 2021 13:04
@ciarmcom ciarmcom added the stale Stale Pull Request label Jun 25, 2021
@ciarmcom
Copy link
Member

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

@ciarmcom ciarmcom added stale Stale Pull Request and removed stale Stale Pull Request labels Jun 25, 2021
@boraozgen
Copy link
Contributor Author

I believe the tests are passing. Needs review.

@0xc0170 0xc0170 added needs: review and removed needs: work stale Stale Pull Request labels Jun 28, 2021
@0xc0170
Copy link
Contributor

0xc0170 commented Jun 28, 2021

@ARMmbed/mbed-os-connectivity Please review

@mergify mergify bot added needs: CI and removed needs: review labels Jun 28, 2021
@0xc0170
Copy link
Contributor

0xc0170 commented Jun 29, 2021

CI started

@mbed-ci
Copy link

mbed-ci commented Jun 29, 2021

Jenkins CI Test : ✔️ SUCCESS

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

CLICK for Detailed Summary

jobs Status
jenkins-ci/mbed-os-ci_unittests ✔️
jenkins-ci/mbed-os-ci_build-cloud-example-ARM ✔️
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-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_build-greentea-ARM ✔️
jenkins-ci/mbed-os-ci_build-greentea-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_cmake-example-ARM ✔️
jenkins-ci/mbed-os-ci_cmake-example-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_build-example-ARM ✔️
jenkins-ci/mbed-os-ci_build-example-GCC_ARM ✔️
jenkins-ci/mbed-os-ci_greentea-test ✔️

@0xc0170 0xc0170 merged commit 16acae3 into ARMmbed:master Jun 29, 2021
@mergify mergify bot removed the ready for merge label Jun 29, 2021
@mbedmain mbedmain added release-version: 6.13.0 Release-pending and removed release-type: patch Indentifies a PR as containing just a patch Release-pending labels Jul 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@paul-szczepanek-arm paul-szczepanek-arm paul-szczepanek-arm approved these changes

@0xc0170 0xc0170 0xc0170 left review comments

Assignees
No one assigned
Labels
release-version: 6.13.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@boraozgen @ciarmcom @0xc0170 @mbed-ci @paul-szczepanek-arm @mbedmain