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

Extract HID DeviceInfo from HidController/HidEnumerator #3308

Merged
merged 12 commits into from
Nov 17, 2020
Merged

Extract HID DeviceInfo from HidController/HidEnumerator #3308

merged 12 commits into from
Nov 17, 2020

Conversation

uklotzde
Copy link
Contributor

  • Split mixxx::hid::DeviceInfo/DeviceCategory from HidController/HidEnumerator
  • Move code into reusable functions to reduce code duplication
  • Avoid redundant copying and manual memory management
  • Use available C/C++ standard functions like strnlen() and wcsnlen()
  • Use immutable members where possible
  • Detailed logging during device discovery
  • Remove unused code/members, e.g. m_sUID

build/features.py Outdated Show resolved Hide resolved
@uklotzde
Copy link
Contributor Author

I decided not to target 2.3, because more changes might follow. The HID code of 2.3 will remain as is.

@uklotzde
Copy link
Contributor Author

Clazy failures will be fixed after #3300 has been merged.

@uklotzde
Copy link
Contributor Author

@Be-ing This is also a small step towards replacing the old controller system. By isolating code that will be needed for both the old and the new controller system we can replace the legacy classes step by step.

@Be-ing
Copy link
Contributor

Be-ing commented Nov 17, 2020

Yes I would like to get confirmation there are no regressions testing with real hardware before merging this. @ywwg @JoergAtGithub @fayaaz could one of you test this?

@Be-ing
Copy link
Contributor

Be-ing commented Nov 17, 2020

The code changes look good to me, just add a little more documentation as noted above.

@JoergAtGithub
Copy link
Member

Yes I would like to get confirmation there are no regressions testing with real hardware before merging this. @ywwg @JoergAtGithub @fayaaz could one of you test this?

I tested this PR on Windows7 and I didn't notice a difference. My device appears with usual name in preferences and my mapping still works.

@Be-ing Be-ing merged commit 8abc1a8 into mixxxdj:main Nov 17, 2020
@uklotzde uklotzde deleted the hid_devices branch November 17, 2020 20:51
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.

None yet

4 participants