Skip to content

Fixup #14531: Make Albatross again the last driver in detection order#14686

Merged
michaelDCurran merged 1 commit into
nvaccess:masterfrom
LeonarddeR:albatrosLast
Mar 1, 2023
Merged

Fixup #14531: Make Albatross again the last driver in detection order#14686
michaelDCurran merged 1 commit into
nvaccess:masterfrom
LeonarddeR:albatrosLast

Conversation

@LeonarddeR
Copy link
Copy Markdown
Collaborator

Link to issue number:

None

Summary of the issue:

As part of #14531, I ordered display order in bdDetect alphabetically. However, it seems that querying Albatross first has a major negative impact on detecting other displays using the same Serial to USB converter (see #13045)

Description of user facing changes

Handy Tech displays such as the Modular Evolution instantly auto detect again.

Description of development approach

Restored the order, Albatross is last in the dictionary again. I'm pretty unhappy with how these displays work, but it looks like it's a design thing in the displays itself.

Testing strategy:

Tested that Handy Tech displays are detected immediately.

Known issues with pull request:

No changes to the albatross driver. @burmancomp Would it be possible to provide a follow up for #13045 that decreases MAX_INIT_RETRIES or changes SLEEP_TIMEOUT to a lower value? This driver takes around 8 seconds to find out whether a display is connected or not if it doesn't receive the necessary packets. I assume as the display sends lots of these packets initially, it should be possible to find out whether initialisation worked after at most 5 attempts.

Change log entries:

None needed

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.

@LeonarddeR LeonarddeR requested a review from a team as a code owner March 1, 2023 07:40
@LeonarddeR LeonarddeR requested a review from seanbudd March 1, 2023 07:40
@burmancomp
Copy link
Copy Markdown
Contributor

In the worst case it takes time to detect Albatross:

  • device can be connected with 19200 and 9600 baud rate, driver does not know which one is used so it has to try with both starting 19200 which is the default
  • although Albatross send continuously its initialization packet when there is no connection, it waits approximately 2 seconds before sending packets if there was working connection. This means that when driver or NVDA are restarted there must be enough time/retries so that device has surely started to send initialization packets
  • I/O buffer reset seems to be necessary for proper operation (this might not be needed if Albatross were first in the list but it would require careful testing), and one reset seems not to be enough (maybe because there are several buffers between NVDA and device).

From constants.py:

MAX_INIT_RETRIES = 20 """ How many times port initialization or opening is tried. This large value is required for braille to start. This should not effect other devices with the same PID and VID, because this device is detected as last one. """

I think it's better to detect Albatross after other devices with same PID and VID.

@LeonarddeR
Copy link
Copy Markdown
Collaborator Author

Thanks for the quick clarification. Let's go ahead with this then and restore the old behavior.

@michaelDCurran michaelDCurran merged commit e8b5385 into nvaccess:master Mar 1, 2023
@nvaccessAuto nvaccessAuto added this to the 2023.2 milestone Mar 1, 2023
@LeonarddeR LeonarddeR deleted the albatrosLast branch August 23, 2025 06:27
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.

4 participants