Skip to content

Interrupt port initialization when there is IOError exception#15239

Merged
seanbudd merged 15 commits into
nvaccess:masterfrom
burmancomp:albatrossInitPort
Aug 22, 2023
Merged

Interrupt port initialization when there is IOError exception#15239
seanbudd merged 15 commits into
nvaccess:masterfrom
burmancomp:albatrossInitPort

Conversation

@burmancomp

@burmancomp burmancomp commented Aug 1, 2023

Copy link
Copy Markdown
Contributor

Link to issue number:

Fixes #15226

Summary of the issue:

Driver continued port initialization retries although there was IOError exception.

Description of user facing changes

Port initialization is stopped when there is IOError exception.

Description of development approach

If port initialization fails, _initPort function raises IOError exception which causes _initConnection function to raise RuntimeError. No need to try with 9600 bps, and there is no other port to try.

Testing strategy:

Tested with Albatross 80.

Known issues with pull request:

none

Change log entries:

New features
Changes
Bug fixes
Fixed bug where Albatross braille displays try to initialize although another braille device has been connected.
For Developers

Code Review Checklist:

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

@burmancomp burmancomp requested a review from a team as a code owner August 1, 2023 06:52
@burmancomp burmancomp requested a review from seanbudd August 1, 2023 06:52
@LeonarddeR

Copy link
Copy Markdown
Collaborator

Thanks @burmancomp I won't be able to test this before monday as I have no display at hand until then.
I"m pretty sure that for my specific case, error 0X20 is involved. I will collect some logs on monday.

@LeonarddeR

Copy link
Copy Markdown
Collaborator

What are actually the cases where an IOError should continue the connection loop? I think I'd prefer an approach where an exception stops connecting unless, instead of an approach where an exception continues the connection loop unless.

@burmancomp burmancomp changed the title Interrupt port initialization when port is busy or there is timeout Interrupt port initialization when there is IOError exception Aug 1, 2023
Comment thread source/brailleDisplayDrivers/albatross/driver.py Outdated
seanbudd
seanbudd previously approved these changes Aug 18, 2023
@burmancomp

Copy link
Copy Markdown
Contributor Author

I removed for loop, I feel it was there just for test/development reasons.

@seanbudd seanbudd dismissed their stale review August 18, 2023 07:03

stale - force push was applied

@AppVeyorBot

Copy link
Copy Markdown

See test results for failed build of commit b89082fc16

@AppVeyorBot

Copy link
Copy Markdown

See test results for failed build of commit 46b0964ab0

Comment thread source/brailleDisplayDrivers/albatross/driver.py Outdated
Comment thread source/brailleDisplayDrivers/albatross/driver.py Outdated
@seanbudd seanbudd marked this pull request as draft August 21, 2023 06:42
@burmancomp burmancomp marked this pull request as ready for review August 21, 2023 08:30

@seanbudd seanbudd 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.

Thanks @burmancomp

@seanbudd

seanbudd commented Aug 22, 2023

Copy link
Copy Markdown
Member

The change log entry should be user focused. You suggest this fixes #15226. Is the following changelog entry accurate? "Fixed bug where Albatross braille displays fail to initialize if another braille device has been connected."

@XLTechie

XLTechie commented Aug 22, 2023 via email

Copy link
Copy Markdown
Collaborator

@burmancomp

Copy link
Copy Markdown
Contributor Author

Maybe this describes change: "Fixed bug where Albatross braille displays try to initialize although another braille device has been connected." I edited change log entry.

@seanbudd seanbudd merged commit 186a8d7 into nvaccess:master Aug 22, 2023
@nvaccessAuto nvaccessAuto added this to the 2023.3 milestone Aug 22, 2023
@burmancomp burmancomp deleted the albatrossInitPort branch August 22, 2023 06:22
seanbudd pushed a commit that referenced this pull request Aug 24, 2023
Fixes #15226

Summary of the issue:
Driver continued port initialization retries although there was IOError exception.

Description of user facing changes
Port initialization is stopped when there is IOError exception.

Description of development approach
If there is IoError exception, _initPort function returns None which causes _initConnection function to raise RuntimeError. No need to try with 9600 bps, and there is no other port to try.
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.

Albatross driver doesn't respect already connected displays

6 participants