I think there could be a more generic way to do this in bdDetect. Actually, taking the bus reported device description into account is pretty neat, because that ensures bdDetect can filter out devices earlier.

Hello.

I will test it and return back with results.

It seems bug with false-positive was fixed.

Only one thing, when i am plugging in ESP32 board, sometimes i am getting an error:
ERROR - hwIo.base.IoBase._notifyReceive (14:52:50.630) - hwIo.ioThread.IoThread (22792):
Traceback (most recent call last):
File "hwIo\base.py", line 204, in _notifyReceive
self._onReceive(data)
TypeError: 'NoneType' object is not callable
ERROR - hwIo.base.IoBase._notifyReceive (14:52:50.771) - hwIo.ioThread.IoThread (22792):
Traceback (most recent call last):
File "hwIo\base.py", line 204, in _notifyReceive
self._onReceive(data)
TypeError: 'NoneType' object is not callable

This PR should be merged to 2023.3 release, as it makes screenreader experience very bad when connecting a microcontroller like esp32.

it is not necessary to merge this to 2023.3 as there are several workarounds. You can for example exclude the Albatross driver from automatic detection.

Thank you @beqabeqa473, nice if it fixes issue.

@LeonarddeR do you think that if this (or more generic solution) is merged, albatross detection should not conflict with other drivers anymore?

@LeonarddeR I will be happy to hear from you an answer, how person which can have such device, which is not so difficult to find, could understand that it is because of this particular braille display. Why person should have to go through this journey if we already discovered it and fix is working?
Why person should wait several months to see this in 2024.1?
Why person should be under the risk if they don't know about this problem?

Basically I don't have anything to say about this since I'm not NV Access.
I'm however pretty sure that issue exists since 2023.1 already.

I am also working on a braille driver for a device that unfortunately has a fairly generic VID/PID combination. I would also like to use the bus description there to minimize false positives, so I would also prefer this as a generic solution in bdDetect.

Regarding the discussion of autodetection of the Albatross driver. It is not for me to decide on this, but due to this issue and other issues in the past that stem from the weird way an Albatross has to be detected, I would not mind if we exclude it from autodetection by default. But that discussion is clearly outside the scope of this PR.

I also prefer generic solution but it sounds too difficult task for me now.

I will perform some research on it on monday, when I have a device with the same VID/PID at hand.

@beqabeqa473 could you provide a dump of the following from the python console?

import hwPortUtils
list(hwPortUtils.listComPorts())

Converting to draft until the design discussion resolves

@burmancomp You can edit hwPortUtils.py as follows:

Above SPDRP_HARDWAREID = 1, add:

SPDRP_DEVICEDESC = 0

Below entry["friendlyName"] = buf.value, add:

			# Device description
			if not SetupDiGetDeviceRegistryProperty(
				g_hdi,
				ctypes.byref(devinfo),
				SPDRP_DEVICEDESC,
				None,
				ctypes.byref(buf), ctypes.sizeof(buf) - 1,
				None
			):
				# Ignore ERROR_INSUFFICIENT_BUFFER
				if ctypes.GetLastError() != ERROR_INSUFFICIENT_BUFFER:
					raise ctypes.WinError()
			else:
				description = entry["deviceDescription"] = buf.value

Then, on the python console:

import hwPortUtils
list(hwPortUtils.listComPorts())

Pretty curious whether the device description for albatross is set correctly, or whether it is just the default USB Serial Port string.

If you mean bus reported device description of usb serial port found in windows device manager (ports (com and lpt)), it is usb serial port.

But bus reported device description of usb serial converter (found in usb drivers) is "Albatross Braille Display".

Thanks, so this is not going to work, we need to look up the bus description of the serial converter instead.
May I ask how you found out the registry query you're using now for fetching the bus description?

In registry editor I searched "Albatross Braille Display", and there was no other entries.

Are there any news with this?

Hello.

I will test it out once it builds

I hope this is fixed for now. I was not able to reproduce this with several nvda restarts with device plugged in, connections/disconnections. Port is beeing logged in log but is not detected as albatros device.

There are some errors in hwio like:
registering pre_configSave action: <class 'brailleDisplayDrivers.handyTech.BrailleDisplayDriver'>
DEBUGWARNING - brailleDisplayDrivers.handyTech.BrailleDisplayDriver._handleInputStream (15:29:48.474) - hwIo.ioThread.IoThread (23828):
Unhandled packet of type b'\x8d'
DEBUGWARNING - brailleDisplayDrivers.handyTech.BrailleDisplayDriver._handleInputStream (15:29:48.474) - hwIo.ioThread.IoThread (23828):
Unhandled packet of type b'\x08'
DEBUGWARNING - brailleDisplayDrivers.handyTech.BrailleDisplayDriver._handleInputStream (15:29:48.474) - hwIo.ioThread.IoThread (23828):
Unhandled packet of type b'\x18'
ERROR - hwIo.base.IoBase._notifyReceive (15:29:48.474) - hwIo.ioThread.IoThread (23828):
Traceback (most recent call last):
File "hwIo\base.pyc", line 204, in _notifyReceive
TypeError: 'NoneType' object is not callable
and one more which unfortunately i have lost, but i am not sure it is in scope of this.

Thanks you @burmancomp and @LeonarddeR for fixes.

@burmancomp - this needs to be rebased on beta and retarget beta if you wish to include in 2024.1. Note, this isn't considered a high priority fix, and is welcome to target 2024.2.
The only reason this can be included in 2024.1 is it is low risk, and we are early enough in the beta cycle for 2024.1.

@burmancomp - this needs to be rebased on beta and retarget beta if you wish to include in 2024.1. Note, this isn't considered a high priority fix, and is welcome to target 2024.2. The only reason this can be included in 2024.1 is it is low risk, and we are early enough in the beta cycle for 2024.1.

@beqabeqa473 what do you think?

@seanbudd let me know if there is something I should do as to rebasing/retargeting if decision is to take this to 2024.1.

I think it would be nice to get this fix in, particularly if you can rebase in the next week.
It's a small and safe bug fix, which we usually allow early in the beta cycle

I have no experience about that process. Is there any documentation?

You need to perform git rebase -i beta. I'd encourage reading the following:

GitHub docs (good background): https://docs.github.com/en/get-started/using-git/about-git-rebase

Manual: https://git-scm.com/docs/git-rebase

Atlassian docs (more details, however have some inaccessible diagrams): https://www.atlassian.com/git/tutorials/rewriting-history/git-rebase

Something like this?

git checkout origin/beta
git branch -u nvaccess/master
git checkout esp32IsNotHandledAsAlbatrossDisplay
git rebase -i beta
git checkout beta
git push origin beta

What to do with comments where main branch is merged to esp32IsNotHandledAsAlbatrossDisplay? Could they be deleted or should some of them included like them which fix merge conflicts? And should all commits squashed/fixed so that result is single commit?

If nvaccess is your origin for this repo, this should be the easiest method.
I'd encourage doing this in VS Code with GitLens however I'm not certain on the accessibility of this.

First, rebase on master, this will make the rebase on beta easier.
Follow this guide on squash rebasing.

git fetch nvaccess
git checkout esp32IsNotHandledAsAlbatrossDisplay
git rebase --keep-base -i nvaccess/master

You'll want to "pick" the first commit with P.
The rest of the commits after use "f" for fix-up.

Now rebase on beta

git fetch nvaccess
git checkout esp32IsNotHandledAsAlbatrossDisplay
git rebase -i nvaccess/beta

You'll want to "drop" everything from master, (e.g. the "Start 2024.2" commit).
Only "pick" the commit you just created.

i think it should be included in 2024.1 to eliminate possible problems which could be serious if someone stucks with it.

I have now hopefully done what @seanbudd adviced in this comment.

Is next step something like:

git push origin/beta esp32IsNotHandledAsAlbatrossDisplay

You have to do a git push -f at the end. then you commits should go to Beta directly if your local branch is properly rebased.

But i advice to do a

git dif remotes/nvaccess/beta burmancomp:esp32IsNotHandledAsAlbatrossDisplay

before pushing, to see what exactly your push will do to the beta branch.

I checked with:

git diff nvaccess/beta esp32IsNotHandledAsAlbatrossDisplay

As far as I understand diff showed that lines this pr adds would be added.

but

git diff nvaccess/beta origin/esp32IsNotHandledAsAlbatrossDisplay

shows a lot of differences. May this be because I have not pushed esp32IsNotHandledAsAlbatrossDisplay after rebasing?

In your case origin is the upstream repository you forged from NV Access and remote is the original NV Access repo, not the copy of it. So that's probably why you see the differences.

There is pull request #15928 now.

I pushed with:

git push origin esp32IsNotHandledAsAlbatrossDisplay:beta

Closing in favour of #15928