• 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/0p5p21mw2yfqtpp5/artifacts/output/nvda_snapshot_pr16340-31523,0b632001.exe
• CI timing (mins):
  INIT 0.0,
  INSTALL_START 1.1,
  INSTALL_END 1.0,
  BUILD_START 0.0,
  BUILD_END 32.0,
  TESTSETUP_START 0.0,
  TESTSETUP_END 0.4,
  TEST_START 0.0,
  TEST_END 2.5,
  FINISH_END 0.2

See test results for failed build of commit 0b6320011c

1. As far as I understand the product, you can choose two connection modes, either HID or Serial. Furthermore, for countries that don't want to use HID, you hide the HID method. Is that correct?
2. If the desired operating mode is Serial, shouldn't the device reject requests to connect using HID?
3. Should people who don't want to use HID have the possibility to disable HID discoverability of the device altogether?

@LeonarddeR
Your points accurately. However, the current issue is that this device cannot hide the HID mode.

1. Yes, that is correct.
2. Currently, the system team has stated that it is challenging to implement a method to reject HID connection requests.
3. As it stands, when the device is connected via USB, the HID connection is inevitably established. It would be ideal to disable it, but currently, it seems difficult.

Given these issues, there is a need for a software solution. Consequently, my current plan is to modify the code to check via Serial if HID is connectable in software terms. Would this be acceptable?

If you could come up with a solution that doesn't touch ordering for other drivers, feel free to give it a try, though I"m afraid that it will be almost impossible to do so given how the detection system functions.
Have you considered leaving out the HID option out of automatic detection?

@LeonarddeR
Yes . But I think that's impossible leaving out. I will commit code tomorrow. Thank you for your help

@EdKweon I've looked briefly at the code and what you're trying to do seems doable but difficult. Here is how I'd go about de-prioritizing HID for a particular driver:

• Add an additional argument (something like useAsAFallBack which should default to False) on DriverRegistrar.addUsbDevices (for consistency the same can be done for addBluetoothDevices, though this is secondary)
• No longer store union of callables and sets of IDs in the _driverDevices for each device - abstract it into a class like DeviceMatcher - it should remember if the given match is a fallback and obviously allow to check if the device in question has a matching ID / Bluetooth name
• Then, in getConnectedUsbDevicesForDriver when iterating through found devices, for each one check if it should be used as a fallback, if so don't yield it immediately, but only at the end when there is no non-fallback match
• Finally, when registering HID ID for Hims, set it as a fallback device, which ensures it will be picked only when serial is not found.

Comments are appreciated.

@seanbudd
I added docstring of FallbackDevicesStore class.

• "can you answer this please?"
  I can't understand what you saying. Can you tell me what I need to respond to?"

@EdKweon - you may need to uncollapse hidden comments. search the page for "6 hidden conversations"

Alternatively, open the PR files page, select conversations, and check through the unresolved comments

@EdKweon - this is looking almost ready other than the last comment. can you do a final round of testing?

@coderabbitai full review

@seanbudd, I've reviewed the PR based on the available information and discussions in the comments. Here are the key points and recommendations:

1. Fallback Device Handling: It's crucial to ensure that the handling of fallback devices is consistent across the functions getConnectedUsbDevicesForDriver and getDriversForConnectedUsbDevices. The modifications should be thoroughly reviewed to confirm that fallback logic works as intended, especially during device disconnections and reconnections.

2. Testing: Given the changes involve critical functionality for device detection and connection stability, a comprehensive testing strategy should be employed. This includes:

   • Unit Tests: Ensure there are unit tests that specifically test the fallback logic under various scenarios, including disconnecting and reconnecting devices.
   • System Tests: Conduct system tests to observe the behavior in a simulated or actual user environment to ensure no regression or unintended behavior.
   • Manual Testing: Manual testing should be conducted to verify the user interface changes, particularly the new options in the Braille Display Select Dialog.

3. Code Review and Documentation: Ensure that all new code is well-documented, especially the parts dealing with fallback devices. This includes updating inline comments and external documentation to reflect the new logic.

4. Linting Issues: Address the linting issues that have caused the build to fail in recent attempts. Ensuring code quality and standards are met will aid in maintaining the codebase health.

Please ensure these areas are addressed comprehensively before finalizing the PR. If there are any specific areas you would like further in-depth analysis or testing on, let me know!

#!/bin/bash
# Fetch the latest commits and show the changes in the files bdDetect.py and hims.py
git log -p -2 -- bdDetect.py hims.py

Thanks @EdKweon