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

drivers/mrf24j40: Expose configurations to Kconfig #13314

Merged

Conversation

leandrolanzieri
Copy link
Contributor

Contribution description

This PR exposes the configurations of the mrf24j40 radio driver to Kconfig. The usage of the options were modified to match the behaviour of Kconfig, so IS_ACTIVE is being used now.

I am setting MRF24J40_USE_EXT_PA_LNA to default y because that's the observed behaviour from drivers/Makefile.dep.

Testing procedure

  • Testing with gnrc_networking and using some mrf24j40 module should be working.
  • Check that if mrj24j40ma is used MRF24J40_USE_EXT_PA_LNA is not available.
  • Changing options from Kconfig should reflect on the application.

Issues/PRs references

Part of #12888

@leandrolanzieri leandrolanzieri added Type: new feature The issue requests / The PR implemements a new feature for RIOT Area: drivers Area: Device drivers TF: Config Marks issues and PRs related to the work of the Configuration Task Force Area: Kconfig Area: Kconfig integration labels Feb 7, 2020
@leandrolanzieri leandrolanzieri force-pushed the pr/kconfig_migrate/drivers/mrf24j40 branch from 4646fc2 to bb56b50 Compare February 7, 2020 10:30
drivers/mrf24j40/Kconfig Outdated Show resolved Hide resolved
@tcschmidt
Copy link
Member

@jia200x @bergzand @fjmolinas any feedback to converge this?

@jia200x
Copy link
Member

jia200x commented Apr 8, 2020

ok, I tested both configuration options:

  • As expected, the node hangs if the test SPI option is disabled when there's no radio.
  • I don't have hardware to test the PA_LNA function, but I put preprocessor errors to check the value is set correctly.

The examples still compile fine, so I would say this PR is in shape

@jia200x
Copy link
Member

jia200x commented Apr 8, 2020

@leandrolanzieri please squash

MRF24J40_TEST_SPI_CONNECTION is moved to the 'CONFIG_' namespace and by
default is not set. Now it is checked in code using 'IS_ACTIVE'.
MRF24J40_USE_EXT_PA_LNA is moved to the 'CONFIG_' namespace and by
default is not set. Now it is checked in code using 'IS_ACTIVE'.
For the mrf24j40 check if Kconfig is being used before setting the
configuration CFLAG.
@leandrolanzieri leandrolanzieri force-pushed the pr/kconfig_migrate/drivers/mrf24j40 branch from e185f41 to 6e49724 Compare April 8, 2020 16:50
@leandrolanzieri
Copy link
Contributor Author

@jia200x squashed

Copy link
Member

@jia200x jia200x left a comment

Choose a reason for hiding this comment

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

It looks good to me. ACK

@jia200x
Copy link
Member

jia200x commented Apr 8, 2020

let's wait for Murdock and travis

@jia200x jia200x added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Apr 8, 2020
@leandrolanzieri leandrolanzieri added this to the Release 2020.04 milestone Apr 8, 2020
@jia200x
Copy link
Member

jia200x commented Apr 8, 2020

all right, let's go!

@jia200x jia200x merged commit bb4e34c into RIOT-OS:master Apr 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: drivers Area: Device drivers Area: Kconfig Area: Kconfig integration CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR TF: Config Marks issues and PRs related to the work of the Configuration Task Force Type: new feature The issue requests / The PR implemements a new feature for RIOT
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants