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

Fix bug in bdDetect device registration affecting Albatross #17585

Merged
merged 2 commits into from
Jan 7, 2025

Conversation

LeonarddeR
Copy link
Collaborator

Link to issue number:

Fixes #17537 (comment)

Summary of the issue:

In #17537, I introduced aa new addUsbDevice method on DriverRegistrar, but it had parameter ordering wrong.

Description of user facing changes

Albatross detection should work again.

Description of development approach

Fixed order. I added a bunch of unit tests to avoid this in the future.

Testing strategy:

Unit tests.

Known issues with pull request:

None known

Code Review Checklist:

  • Documentation:
    • Change log entry
    • User Documentation
    • Developer / Technical Documentation
    • Context sensitive help for GUI changes
  • Testing:
    • Unit tests
    • System (end to end) tests
    • Manual testing
  • UX of all users considered:
    • Speech
    • Braille
    • Low Vision
    • Different web browsers
    • Localization in other languages / culture than English
  • API is compatible with existing add-ons.
  • Security precautions taken.

@coderabbitai summary

@LeonarddeR LeonarddeR requested a review from a team as a code owner January 6, 2025 20:21
@LeonarddeR LeonarddeR requested a review from seanbudd January 6, 2025 20:21
@LeonarddeR
Copy link
Collaborator Author

@burmancomp could you please confirm whether this fixes the issue?

@SaschaCowley SaschaCowley added the conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review. label Jan 6, 2025
tests/unit/test_bdDetect.py Outdated Show resolved Hide resolved
tests/unit/test_bdDetect.py Outdated Show resolved Hide resolved
tests/unit/test_bdDetect.py Outdated Show resolved Hide resolved
Copy link
Member

@seanbudd seanbudd left a comment

Choose a reason for hiding this comment

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

Thanks @LeonarddeR

@seanbudd seanbudd merged commit 1edb075 into nvaccess:master Jan 7, 2025
5 checks passed
@github-actions github-actions bot added this to the 2025.1 milestone Jan 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants