Skip to content

Kconfig: Expose USB configurations#12936

Merged
miri64 merged 8 commits intoRIOT-OS:masterfrom
leandrolanzieri:pr/kconfig_migrate/usb
Jan 14, 2020
Merged

Kconfig: Expose USB configurations#12936
miri64 merged 8 commits intoRIOT-OS:masterfrom
leandrolanzieri:pr/kconfig_migrate/usb

Conversation

@leandrolanzieri
Copy link
Copy Markdown
Contributor

@leandrolanzieri leandrolanzieri commented Dec 12, 2019 •
edited
Loading

Contribution description

This PR moves configuration macros of USB to the CONFIG_ name space and exposes them to Kconfig.

I also added a Kconfig to examples/usb_minimal to have VID/PID defaults only for that application. Currently the PR depends on #12913 (97065a8), because I added a check in the application's Makefile to not set the VID/PID via CFLAGS when Kconfig is already being used for that.

Testing procedure

  • By default examples/usb_minimal should use all the default configurations as always.
  • Running make menuconfig should allow to configure USB. When ran in examples/usb_minimal VID/PID should have default values. If the default values are changed the warning message should not appear.

Issues/PRs references

Depends on #12913
Part of #12888

@leandrolanzieri leandrolanzieri added Type: new feature The issue requests / The PR implemements a new feature for RIOT State: waiting for other PR State: The PR requires another PR to be merged first TF: Config Marks issues and PRs related to the work of the Configuration Task Force Area: USB Area: Universal Serial Bus labels Dec 12, 2019
@leandrolanzieri leandrolanzieri added this to the Release 2020.01 milestone Dec 12, 2019
@leandrolanzieri leandrolanzieri mentioned this pull request Dec 12, 2019
Open
71 tasks
@leandrolanzieri
Copy link
Copy Markdown
Contributor Author

leandrolanzieri commented Dec 19, 2019

Rebased to master, this has no dependencies now.

@leandrolanzieri leandrolanzieri removed the State: waiting for other PR State: The PR requires another PR to be merged first label Dec 19, 2019
bergzand
bergzand previously requested changes Dec 19, 2019
View reviewed changes
@bergzand
Copy link
Copy Markdown
Member

bergzand commented Dec 19, 2019

I'm not confident in reviewing the modifications to the minimal usbus example, maybe somebody else could look into that

bergzand
bergzand reviewed Dec 19, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@bergzand bergzand left a comment

Choose a reason for hiding this comment

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

Some option naming nitpicks.

Is there a consensus (or a rough idea) on a option naming scheme?

@leandrolanzieri
Copy link
Copy Markdown
Contributor Author

leandrolanzieri commented Dec 20, 2019

I'm not confident in reviewing the modifications to the minimal usbus example, maybe somebody else could look into that

Maybe @cgundogan can take a look?

cgundogan
cgundogan reviewed Jan 2, 2020
View reviewed changes
@cgundogan
Copy link
Copy Markdown
Member

cgundogan commented Jan 2, 2020

Maybe @cgundogan can take a look?

the changes to the usbus example app are small .. IMO they look fine. I will run a test today and return with my results.

@cgundogan
Copy link
Copy Markdown
Member

cgundogan commented Jan 7, 2020

the changes to the usbus example app are small .. IMO they look
fine. I will run a test today and return with my results.

I did a small test using a samr21 board and changed the configurations with menuconfig. My system showed the new configurations once I plugged in the user USB into my laptop.

@fjmolinas
Copy link
Copy Markdown
Contributor

fjmolinas commented Jan 8, 2020

@cgundogan @bergzand is this ok to squash?

@tcschmidt
Copy link
Copy Markdown
Member

tcschmidt commented Jan 14, 2020

Ping @bergzand @cgundogan

@bergzand
Copy link
Copy Markdown
Member

bergzand commented Jan 14, 2020

I'm fine with the changes here. I don't have the time at the moment to put this through a test, I have to leave that to the next reviewer :(

@bergzand bergzand added Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines labels Jan 14, 2020
@bergzand bergzand dismissed their stale review January 14, 2020 09:14

Change requests are resolved

@bergzand bergzand added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jan 14, 2020
@cgundogan cgundogan added the Reviewed: 3-testing The PR was tested according to the maintainer guidelines label Jan 14, 2020
cgundogan
cgundogan approved these changes Jan 14, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@cgundogan cgundogan left a comment

Choose a reason for hiding this comment

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

Also OK to squash from my side!

@leandrolanzieri
usb: Move configuration macros to 'CONFIG_' namespace
15bd489 
Macros that changed:
USB_CONFIG_VID -> CONFIG_USB_VID
USB_CONFIG_PID -> CONFIG_USB_PID
USB_CONFIG_MANUF_STR -> CONFIG_USB_MANUF_STR
USB_CONFIG_PRODUCT_STR -> CONFIG_USB_PRODUCT_STR
USB_CONFIG_CONFIGURATION_STR -> CONFIG_USB_CONFIGURATION_STR
USB_CONFIG_PRODUCT_BCDVERSION -> CONFIG_USB_PRODUCT_BCDVERSION
USB_CONFIG_SPEC_BCDVERSION -> CONFIG_USB_SPEC_BCDVERSION
USB_CONFIG_SELF_POWERED -> CONFIG_USB_SELF_POWERED
USB_CONFIG_MAX_POWER -> CONFIG_USB_MAX_POWER
USB_CONFIG_DEFAULT_LANGID -> CONFIG_USB_DEFAULT_LANGID
@leandrolanzieri
Copy link
Copy Markdown
Contributor Author

leandrolanzieri commented Jan 14, 2020

Also OK to squash from my side!

@cgundogan squashed

miri64
miri64 reviewed Jan 14, 2020
View reviewed changes
@miri64 miri64 merged commit 350b33b into RIOT-OS:master Jan 14, 2020
@leandrolanzieri leandrolanzieri deleted the pr/kconfig_migrate/usb branch January 14, 2020 11:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@miri64 miri64 miri64 left review comments

@bergzand bergzand bergzand left review comments

@cgundogan cgundogan cgundogan approved these changes

Assignees

No one assigned

Labels

Area: USB Area: Universal Serial Bus CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines 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

Milestone

Release 2020.01

Development

Successfully merging this pull request may close these issues.

6 participants

@leandrolanzieri @bergzand @cgundogan @fjmolinas @tcschmidt @miri64