bdDetect: Enrich USB serial data with info about USB device #15764

LeonarddeR · 2023-11-09T13:48:02Z

Link to issue number:

Summary of the issue:

Many braille displays use a serial converter from FTDI with VID 0x403" and PID 0x6001. Given the weird nature of Albatross devices not being able to identify themselves on request, the Albatross driver is currently a disturbing factor in automatic detection.

Description of user facing changes

None.

Description of development approach

This pr enriches information for serial ports in bdDetect. The _DeviceInfoFetcher class now has an extra autoprop usbComPorts that bundles com port info with USB metadata. If all works as expected, the Albatross driver will be able to limit itself to Albatross devices only after this pr by using the busReportedDeviceDescription dictionary entry.
Removed duplicated code from hwPortUtils

Testing strategy:

Testing with Albatross from @burmancomp would be appreciated.
It can be tested using bdDetect.getConnectedUsbDevicesForDriver("albatross"). This should yield device matches with details about the underlying USB device.

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.

burmancomp · 2023-11-09T18:46:36Z

Yes, I am about to test this likely tomorrow.

burmancomp · 2023-11-10T13:22:12Z

Portable version of nvda_snapshot_pr15764-29937,d9a7c5be.exe, python console:

next(bdDetect.getConnectedUsbDevicesForDriver("albatross"))
DeviceMatch(type='serial', id='VID_0403&PID_6001', port='COM5', deviceInfo={'hardwareID': 'USB\VID_0403&PID_6001&REV_0400', 'port': 'COM5', 'usbID': 'VID_0403&PID_6001', 'friendlyName': 'USB Serial Port (COM5)', 'devicePath': '\\?\usb#vid_0403&pid_6001#tmboijcd#{a5dcbf10-6530-11d2-901f-00c04fb951ed}', 'deviceDescription': 'USB Serial Converter'})
Let me know did I test correctly?

LeonarddeR · 2023-11-10T14:00:44Z

Yes, this is correct. I need to expand hwPortUtils but I at least know how.
Could you by any chance do the following?

Open Device Manager
Open ports (com & lpt)
Select your com5 port
Look up de device details and in there, the Bus reported device description
I assume that's USB Serial Port, right?

burmancomp · 2023-11-10T15:01:18Z

Yes, it is "USB Serial Port", and usb serial converter bus reported device description is "Albatross Braille Display".

Let me know if you need any additional information.

LeonarddeR · 2023-11-10T18:22:21Z

I think the most recent commit should finally give us what's needed, namely Albatross Braille Display in the busReportedDeviceDescription

burmancomp · 2023-11-10T21:11:52Z

Unfortunately albatross did not start, and it is not present in list of displays.

import bdDetect
next(bdDetect.getConnectedUsbDevicesForDriver("albatross"))
Traceback (most recent call last):
File "", line 1, in
File "bdDetect.pyc", line 441, in getConnectedUsbDevicesForDriver
File "baseObject.pyc", line 62, in get
File "baseObject.pyc", line 168, in _getPropertyViaCache
File "bdDetect.pyc", line 245, in _get_usbDevices
File "hwPortUtils.pyc", line 424, in listUsbDevices
OSError: [WinError 1168] Elementtiä ei löydy.

LeonarddeR · 2023-11-11T03:44:41Z

Have you performed the test step on the python console?

burmancomp · 2023-11-11T08:23:16Z

Yes I tested in python console getting those result.

LeonarddeR · 2023-11-11T08:39:38Z

Ah I'm sorry, somehow I didn't get your test result.

LeonarddeR · 2023-11-11T08:44:42Z

Pretty sure last commit eventually should get you going

burmancomp · 2023-11-13T11:06:50Z

congratulations @LeonarddeR!

`>>> import bdDetect

next(bdDetect.getConnectedUsbDevicesForDriver("albatross"))
DeviceMatch(type='serial', id='VID_0403&PID_6001', port='COM5', deviceInfo={'hardwareID': 'USB\VID_0403&PID_6001&REV_0400', 'port': 'COM5', 'usbID': 'VID_0403&PID_6001', 'friendlyName': 'USB Serial Port (COM5)', 'devicePath': '\\?\usb#vid_0403&pid_6001#tmboijcd#{a5dcbf10-6530-11d2-901f-00c04fb951ed}', 'busReportedDeviceDescription': 'Albatross Braille Display'})`

I am about to close #15693.

LeonarddeR · 2023-11-13T11:48:29Z

Note that this pr doesn't yet contain a workaround for Albatross. Therefore I'd leave #15693 open and adapt it after merge of this pr.

burmancomp · 2023-11-13T16:26:52Z

Thank you for information. I leave it open.

burmancomp · 2023-11-13T17:11:34Z

Thank you for information. I leave it open.

ps. After reading your previous comment and then inspecting your initial comment I realized that your thought is that albatross driver is changed too. I realized it before writing my previous comment (which I am quoting in this comment).

LeonarddeR · 2023-11-13T17:56:32Z

This pr does not yet improve the Albatross driver by itself.

burmancomp · 2023-11-13T18:37:38Z

But albatross driver can query bus reported device description?

LeonarddeR · 2023-11-13T20:41:29Z

Sure, after this is merged.

source/hwPortUtils.py

seanbudd · 2023-11-14T02:48:43Z

source/hwPortUtils.py

+				"devicePath": idd.DevicePath
+			})
+			if _isDebug():
+				log.debug("%r" % usbId)


can this use an f-string

this is unresolved

source/hwPortUtils.py

user_docs/en/changes.t2t

Co-authored-by: Sean Budd <seanbudd123@gmail.com>

…into enrichtSerialUsb

AppVeyorBot · 2023-11-15T19:15:38Z

Build (for testing PR): https://ci.appveyor.com/api/buildjobs/yowun8m4brq8n4yo/artifacts/output/
CI timing (mins):
INIT 0.0,
INSTALL_START 1.0,
INSTALL_END 1.0,
BUILD_START 0.0,
FINISH_END 26.4

See test results for failed build of commit c9ce3f98b6

LeonarddeR · 2023-11-15T20:06:15Z

The only way I could get rid of the c901 was by rewriting the if/elif statements into a match statement.

lukaszgo1 · 2023-11-18T16:13:57Z

The only way I could get rid of the c901 was by rewriting the if/elif statements into a match statement.

That works only because match and case statements are not measured for complexity by mccabe, so the function is as complex as before, the only change is that complexity is not checked for a language construct used. If it is indeed so hard to simplify it perhaps the noqa comment should remain, so that it is clear we haven't really gained anything here?

LeonarddeR · 2023-11-18T18:11:52Z

In that case I prefer to revert the match statement.

seanbudd · 2023-11-21T23:50:15Z

so the function is as complex as before

I don't think this is true - the function was still split up into listComPorts/_getBluetoothPortInfo which reduced complexity in listComPorts significantly, and I am fairly sure to the point of legitimate c901 removal in both functions.

source/hwPortUtils.py

seanbudd · 2023-11-21T23:55:42Z

source/hwPortUtils.py

+				"devicePath": idd.DevicePath
+			})
+			if _isDebug():
+				log.debug("%r" % usbId)


this is unresolved

Co-authored-by: Sean Budd <seanbudd123@gmail.com>

AppVeyorBot · 2023-11-22T07:28:30Z

PASS: Translation comments check.
PASS: Unit tests.
FAIL: Lint check. See test results for more information.
PASS: System tests (tags: installer NVDA).
Build (for testing PR): https://ci.appveyor.com/api/buildjobs/t2lonv34od1860jy/artifacts/output/nvda_snapshot_pr15764-30012,1c32201d.exe
CI timing (mins):
INIT 0.0,
INSTALL_START 0.9,
INSTALL_END 1.0,
BUILD_START 0.0,
BUILD_END 26.7,
TESTSETUP_START 0.0,
TESTSETUP_END 0.3,
TEST_START 0.0,
TEST_END 16.6,
FINISH_END 0.1

See test results for failed build of commit 1c32201dea

seanbudd

Thanks @LeonarddeR

LeonarddeR added 4 commits November 9, 2023 14:17

Dramatically less complex hwPortUtils code

31305ce

Update bdDetect

2f831af

Lint

a95bbb6

Changelog

52436fd

LeonarddeR added 2 commits November 10, 2023 08:08

use a separate auto prop on the device cache

0825e64

Merge remote-tracking branch 'origin/master' into enrichtSerialUsb

d1b972c

Add busReportedDeviceDescription

41f167d

Catch error when no bus description is available

e5939ed

LeonarddeR marked this pull request as ready for review November 13, 2023 11:48

LeonarddeR requested a review from a team as a code owner November 13, 2023 11:48

LeonarddeR requested a review from seanbudd November 13, 2023 11:48

seanbudd reviewed Nov 14, 2023

View reviewed changes

Apply suggestions from code review

9af6394

Co-authored-by: Sean Budd <seanbudd123@gmail.com>

LeonarddeR marked this pull request as draft November 14, 2023 06:51

LeonarddeR added 3 commits November 15, 2023 19:43

more linting

2c157dd

Merge branch 'enrichtSerialUsb' of https://github.com/leonardder/nvda …

4cc8221

…into enrichtSerialUsb

Merge remote-tracking branch 'origin/master' into enrichtSerialUsb

838436a

use a special function

fbb4e87

LeonarddeR marked this pull request as ready for review November 15, 2023 20:07

seanbudd added the conceptApproved Similar 'triaged' for issues, PR accepted in theory, implementation needs review. label Nov 21, 2023

seanbudd reviewed Nov 22, 2023

View reviewed changes

LeonarddeR and others added 3 commits November 22, 2023 07:39

Merge remote-tracking branch 'origin/master' into enrichtSerialUsb

639296a

Use F-string in log statement

81ee6bb

Update source/hwPortUtils.py

6e4220a

Co-authored-by: Sean Budd <seanbudd123@gmail.com>

Typo fixes

1d43337

seanbudd approved these changes Nov 22, 2023

View reviewed changes

seanbudd added the api-breaking-change label Nov 22, 2023

seanbudd merged commit 5669fdf into nvaccess:master Nov 23, 2023
1 check passed

nvaccessAuto added this to the 2024.1 milestone Nov 23, 2023

burmancomp added a commit to burmancomp/nvda that referenced this pull request Nov 23, 2023

modified to use busReportedDeviceDescription from nvaccess#15764

3e9e313

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bdDetect: Enrich USB serial data with info about USB device #15764

bdDetect: Enrich USB serial data with info about USB device #15764

LeonarddeR commented Nov 9, 2023 •

edited

burmancomp commented Nov 9, 2023

burmancomp commented Nov 10, 2023

LeonarddeR commented Nov 10, 2023

burmancomp commented Nov 10, 2023

LeonarddeR commented Nov 10, 2023

burmancomp commented Nov 10, 2023

LeonarddeR commented Nov 11, 2023 via email

burmancomp commented Nov 11, 2023

LeonarddeR commented Nov 11, 2023

LeonarddeR commented Nov 11, 2023

burmancomp commented Nov 13, 2023 •

edited

LeonarddeR commented Nov 13, 2023

burmancomp commented Nov 13, 2023 •

edited

burmancomp commented Nov 13, 2023

LeonarddeR commented Nov 13, 2023

burmancomp commented Nov 13, 2023

LeonarddeR commented Nov 13, 2023

seanbudd Nov 14, 2023

seanbudd Nov 21, 2023

AppVeyorBot commented Nov 15, 2023

LeonarddeR commented Nov 15, 2023

lukaszgo1 commented Nov 18, 2023

LeonarddeR commented Nov 18, 2023

seanbudd commented Nov 21, 2023

seanbudd Nov 21, 2023

AppVeyorBot commented Nov 22, 2023

seanbudd left a comment

bdDetect: Enrich USB serial data with info about USB device #15764

bdDetect: Enrich USB serial data with info about USB device #15764

Conversation

LeonarddeR commented Nov 9, 2023 • edited

Link to issue number:

Summary of the issue:

Description of user facing changes

Description of development approach

Testing strategy:

Known issues with pull request:

Code Review Checklist:

burmancomp commented Nov 9, 2023

burmancomp commented Nov 10, 2023

LeonarddeR commented Nov 10, 2023

burmancomp commented Nov 10, 2023

LeonarddeR commented Nov 10, 2023

burmancomp commented Nov 10, 2023

LeonarddeR commented Nov 11, 2023 via email

burmancomp commented Nov 11, 2023

LeonarddeR commented Nov 11, 2023

LeonarddeR commented Nov 11, 2023

burmancomp commented Nov 13, 2023 • edited

LeonarddeR commented Nov 13, 2023

burmancomp commented Nov 13, 2023 • edited

burmancomp commented Nov 13, 2023

LeonarddeR commented Nov 13, 2023

burmancomp commented Nov 13, 2023

LeonarddeR commented Nov 13, 2023

seanbudd Nov 14, 2023

Choose a reason for hiding this comment

seanbudd Nov 21, 2023

Choose a reason for hiding this comment

AppVeyorBot commented Nov 15, 2023

LeonarddeR commented Nov 15, 2023

lukaszgo1 commented Nov 18, 2023

LeonarddeR commented Nov 18, 2023

seanbudd commented Nov 21, 2023

seanbudd Nov 21, 2023

Choose a reason for hiding this comment

AppVeyorBot commented Nov 22, 2023

seanbudd left a comment

Choose a reason for hiding this comment

LeonarddeR commented Nov 9, 2023 •

edited

burmancomp commented Nov 13, 2023 •

edited

burmancomp commented Nov 13, 2023 •

edited