Skip to content

Follow-up of #13045: Removed redundant \xfe\xfd from ESTABLISHED constant#14705

Merged
seanbudd merged 1 commit into
nvaccess:masterfrom
burmancomp:fixAlbatrossConnectionEstablishmentString
Mar 7, 2023
Merged

Follow-up of #13045: Removed redundant \xfe\xfd from ESTABLISHED constant#14705
seanbudd merged 1 commit into
nvaccess:masterfrom
burmancomp:fixAlbatrossConnectionEstablishmentString

Conversation

@burmancomp
Copy link
Copy Markdown
Contributor

@burmancomp burmancomp commented Mar 7, 2023

Link to issue number:

Follow-up of #13045

Summary of the issue:

Driver sent to Albatross display byte string b"\xfe\xfd\xfe\xfd" to tell device that there is active connection and it should not send init packets anymore. Byte string contained two establishment packets, and one should be enough. It should not cause harm to send two packets because display ignores latter one but it is redundant anyway.

In stead of b"\xfe\xfd\xfe\xfd" driver sends now b"\xfe\xfd" to display.

Description of user facing changes

None

Description of development approach

None

Testing strategy:

Starts/restarts, switching display off/on during connection, and using device's internal menu during connection.

Known issues with pull request:

None

Change log entries:

None

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 marked this pull request as ready for review March 7, 2023 22:14
@burmancomp burmancomp requested a review from a team as a code owner March 7, 2023 22:14
@burmancomp burmancomp requested a review from seanbudd March 7, 2023 22:14
@seanbudd
Copy link
Copy Markdown
Member

seanbudd commented Mar 7, 2023

Are there are no changes for the user, what motivates this change?

@burmancomp
Copy link
Copy Markdown
Contributor Author

Are there are no changes for the user, what motivates this change?

In principle there might be minimal effect on how quickly display shows braille because it needs not to handle the other confirmation packet which it seems to skip because it waits after confirmation packet for different data like something to show in braille. I noticed this when I read display firmware code, so why to send such data what is not used supposing that first packet reaches display.

@seanbudd seanbudd merged commit fa89f05 into nvaccess:master Mar 7, 2023
@nvaccessAuto nvaccessAuto added this to the 2023.2 milestone Mar 7, 2023
@burmancomp
Copy link
Copy Markdown
Contributor Author

Thank you!

@burmancomp burmancomp deleted the fixAlbatrossConnectionEstablishmentString branch March 8, 2023 00:01
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.

3 participants