Skip to content

Bugfix for Braille display detection#18115

Merged
seanbudd merged 4 commits into
nvaccess:betafrom
christiancomaschi:patch-1
May 19, 2025
Merged

Bugfix for Braille display detection#18115
seanbudd merged 4 commits into
nvaccess:betafrom
christiancomaschi:patch-1

Conversation

@christiancomaschi

Copy link
Copy Markdown
Contributor

Link to issue number:

closes #18114

Summary of the issue:

Braille display sometimes failing to be detected

Description of user facing changes

Description of development approach

I read the log file and found that the problem was an incorrect use of sets while populating the fallback drivers list

Testing strategy:

I replaced BdDetect.py with my fixed version in the latest beta portable version, restarted NVDA and now it seems to work and doesn't raise any TypeError

Known issues with pull request:

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

@christiancomaschi christiancomaschi requested a review from a team as a code owner May 15, 2025 10:55

@LeonarddeR LeonarddeR left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Interesting bug and good catch!

@LeonarddeR LeonarddeR left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I was too quick. Could you also update the changelog for this?
I think it might make sense to have this in NVDA 2025.1, but I leave it up to NV Access to decide so.

@christiancomaschi

Copy link
Copy Markdown
Contributor Author

I was too quick. Could you also update the changelog for this? I think it might make sense to have this in NVDA 2025.1, but I leave it up to NV Access to decide so.

It's my first pull requqest and I'm not finding any Changelog.md to edit, do you mean the changelog section in the PR? Is there a specific format or is it enough to write a small sentence like "Fixed Braille display detection"?

@LeonarddeR

Copy link
Copy Markdown
Collaborator

Could you please edit the file user_docs/en/changes.md and add the changelog entry under bug fixes?

@christiancomaschi

Copy link
Copy Markdown
Contributor Author

Could you please edit the file user_docs/en/changes.md and add the changelog entry under bug fixes?

Ok I have changed the file in the section of NVDA 2025.1 as you suggested, is it ok or should I move the change to 2025.2?

Comment thread user_docs/en/changes.md Outdated
Comment thread source/bdDetect.py
christiancomaschi and others added 4 commits May 19, 2025 11:05
Quick fix for an error when adding a device to the fallback list due to an incorrect use of sets
Co-authored-by: Sascha Cowley <16543535+SaschaCowley@users.noreply.github.com>
@SaschaCowley SaschaCowley changed the base branch from master to beta May 19, 2025 01:07

@SaschaCowley SaschaCowley left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@seanbudd seanbudd added this to the 2025.1 milestone May 19, 2025
@seanbudd seanbudd merged commit b82f75f into nvaccess:beta May 19, 2025
3 checks passed
@sharkboyto

Copy link
Copy Markdown

I have a Focus braille display from Freedom Scientific, connected via USB. The display works with Jaws, but it doesn't work with NVDA.
In the last few days, I've tried NVDA beta 6 and beta 8, but the braille display still doesn't work.
I'm attaching the log file.

nvda-log.txt

@LeonarddeR

Copy link
Copy Markdown
Collaborator

@sharkboyto I'm pretty sure that issue is not related, please open a new issue for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

HIMS Braille Emotion display sometimes failing to be detected when connected to USB

5 participants