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

STM32WL LORA: HW specific out of STM32WL_LoRaRadio class #14734

Merged
merged 1 commit into from
Jun 11, 2021

Conversation

jeromecoutant
Copy link
Collaborator

Summary of changes

Some code in STM32WL_LoRaRadio class was specific to NUCLEO_WL55JC HW,
and not general to STM32WL

It is then now compiled in another file.
Function is weak, and can be / has to be updated in each HW application

Should fix #14714

@niondir @ludoch-stm

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

[] 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 Jun 4, 2021
@ciarmcom
Copy link
Member

ciarmcom commented Jun 4, 2021

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

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.

is weak function really needed ? Seems like based on the state that is valid for everyone, just the ctrl1-3 changes its values. or is there more that can come in?

wondering if config would be sufficient for this or we need a weakly function

@jeromecoutant
Copy link
Collaborator Author

is weak function really needed ? Seems like based on the state that is valid for everyone, just the ctrl1-3 changes its values. or is there more that can come in?
wondering if config would be sufficient for this or we need a weakly function

I have discussed with some LORA experts, and I understand that our HW reference design currently supports 4 antenna switch state, which are then configurable thanks to 3 GPIOs.
But I don't know at all, beut a custom board can maybe supports only 3 states ?
So how to do the mapping between available GPIO and state ?

@0xc0170
Copy link
Contributor

0xc0170 commented Jun 7, 2021

@niondir what your custom function look like, if you just use one pin ?

@ciarmcom ciarmcom added the stale Stale Pull Request label Jun 10, 2021
@ciarmcom
Copy link
Member

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

@mergify mergify bot added needs: CI and removed needs: review labels Jun 10, 2021
@ciarmcom ciarmcom removed the stale Stale Pull Request label Jun 10, 2021
@0xc0170
Copy link
Contributor

0xc0170 commented Jun 11, 2021

CI started

@mbed-ci
Copy link

mbed-ci commented Jun 11, 2021

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

@0xc0170 0xc0170 merged commit 530d4be into ARMmbed:master Jun 11, 2021
@mergify mergify bot removed the ready for merge label Jun 11, 2021
@jeromecoutant jeromecoutant deleted the PR_RFSWITCH branch June 11, 2021 14:10
@niondir
Copy link

niondir commented Jun 14, 2021

@0xc0170
Looks fine with the weak function - so I can just override it. I actually use 2 pins now. One is "RF-Switch Power on/off" and one is for the direction (RX/TX). I was able to use 2 of the existing pins, since they work as expected and set the third one to an unused PIN.

With this fix I can simply build my own method just containing my pins. That's great.

Kind regards
Tobias

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.

Support stm32wl-lora-driver.rf-switch-ctl1..3 as NC
6 participants