ZHA USB discovery talks on connected serial ports by default

[oxan]

The problem

The ZHA USB discovery added in #54935 will talk on serial ports in a default installation (I used a clean config), losing communication with the device for the application that was using it and possibly breaking any devices that are connected. I had an ESPHome device connected that somehow ended up in flash mode.

I think the main problem here is the VID/PID combinations listed in the ZHA manifest are too generic. E.g. the 1A86:7523 combination is for the CH340 USB to serial chip, which is a generic serial-to-USB chip that I bet is used in more non-Zigbee-related products than in Zigbee-dongles. Likewise the 10C4:EA60 device listed is for CP201x, another common serial-to-USB chip.

Personally I would strongly suggest not writing anything to such serial ports by default, but there should at least be an option to disable it.

What is version of Home Assistant Core has the issue?

dev

What was the last working version of Home Assistant Core?

2021.8.8

What type of installation are you running?

Home Assistant Core

Integration causing the issue

zha, usb

Link to integration documentation on our website

https://www.home-assistant.io/integrations/zha/

Example YAML snippet

No response

Anything in the logs that might be useful for us?

2021-08-25 21:53:32 INFO (SyncWorker_0) [homeassistant.loader] Loaded zha from homeassistant.components.zha
2021-08-25 21:53:42 WARNING (MainThread) [zigpy_deconz.uart] Invalid checksum: 0xe011, data: 0x6966693a3336305d3a202020495020827d190b71e26143d7a87175e76031
2021-08-25 21:53:42 WARNING (MainThread) [zigpy_deconz.uart] Invalid checksum: 0x19e2, data: 0xc772b1396171b8a8b8f075e7f167603a709086d7a8f175e77031201d7dec3360f28de7f0f0e0f0e7b81d38e1f0c7e771676072b2b1f13de1f0c7e7613120c78843d7a87175e76131a03a32b8b170e267b8f08ef27c6065cab14f4f7143d7a87175e771316879e1e289c138e1f0c7e7716760e331b2330d38e1f0f8eae76031af1d8079a833f0c7f2b0d0e8d482651d7938e18dc7f2b031203219c779
2021-08-25 21:53:42 WARNING (MainThread) [zigpy_deconz.uart] Invalid checksum: 0xc833, data: 0x387763c7ebe067757294d880191d329de8e8c238e1f0f8ea8b7067753238f3d4d4ea5d33a8f3a818ea8b0b3111e0c8b07062f4703d
2021-08-25 21:53:42 WARNING (MainThread) [zigpy_deconz.uart] Invalid checksum: 0x3dcc, data: 0x606060626243d7a8f175e7b0f1e767b8f075f25c604532a8ec1905f838e1f0c7f25c604775fa190b4f71e067308dc7f27c60c442777d65e065f286c738e1f0c7e7f167603da3c4c77d5d7763c7f2703120ccb07a99bcf0e0f0bc585abaf1e7b8f85d77e3c7e76031e0f2700d5d7763c7f2613120
2021-08-25 21:53:42 WARNING (MainThread) [zigpy_deconz.uart] Invalid checksum: 0xf1f1, data: 0x38e18dc7f27c60f5bbf162
2021-08-25 21:53:42 WARNING (MainThread) [zigpy_deconz.uart] Invalid checksum: 0x8af0, data: 0x38e1f075e7f167e0ab4be1e2f2e267388dc7f25c60d40f0b4f7143d7a87175e7713168791cd4c8a8f3e77175f2f83af174
2021-08-25 21:53:43 WARNING (MainThread) [zigpy_deconz.uart] Invalid checksum: 0x1d70, data: 0xe1387763c7caf0e760676033750543c4510d38e1f0c7f2f83af167e0baccb17030e338fda8f175caf03af17445f1b0e138e1f075f2f83af1e8307271623043c445f25d7763c7caf0e76067e0baccb17030e338fda8f175caf0e7f17445f1b03738e1f075f2f83af1e830727162304371a2f25d7763c7caf0e76067e0baccb17030e338fda8718ecaf0e7e06745f1b03738e1f0c7caf0e7f1e83072716230faa8a2f238e1f075caf0e7f131e8ea796162e6a8f3a7718ecaf0e7f17445f1b0375d7763c7caf0e7606760478805d2faa8a2f25d7763c7caf0e76067e0ba796160e338fda8f175ca3b6067e42a329067338dc7ca3bf1e8303b28f2f85d7763c7f23ab2b03128f3e8f3
2021-08-25 21:53:43 WARNING (MainThread) [zigpy_deconz.uart] Invalid checksum: 0x7d70, data: 0x38fda8f175eab167f6c8a8338dc7dcca616760ccd48e875767b8f075ea59eb80dc90f21d606043d7a8718eeab1e830f33a3a8e50aaa8f3e7718e90cd3190aa86d7a8f175c7b16760eab2725933ec33b871b8f0c7dc900b312839327d6243d7a87175c7e1676033afca60e267b8f08e900b3128333ab1b0f138fdab718e90cd3138aa86d7a8718ec7606760c83b3233dd28bb33b871d7e77175c7b1e83071a332
2021-08-25 21:53:43 WARNING (MainThread) [zigpy_deconz.uart] Invalid checksum: 0x33c8, data: 0x38fda8718e90cd31c70408e667338dc7dc908d3128b23b323372
2021-08-25 21:53:43 WARNING (MainThread) [zigpy_deconz.uart] Invalid checksum: 0x7d70, data: 0x1538e1f0c773fb0b6065a37a32b8e038fda87175c7e067e63ba87d5d7763c7dc90b13128b23b32337233ec33b871d7a8f175c7b1e830eaa332
2021-08-25 21:53:44 WARNING (MainThread) [zigpy_deconz.api] No response to 'Command.device_state' command with seq id '0x02'
2021-08-25 21:53:44 INFO (MainThread) [zigpy_cc.uart] Bytes received: b"unique_id: 'lightslightrgbww_cil"
2021-08-25 21:53:44 INFO (MainThread) [zigpy_cc.uart] Bytes received: b"'\n  supported_color_modes: COLOR"
2021-08-25 21:53:44 INFO (MainThread) [zigpy_cc.uart] Bytes received: b'_MODE_COLD_WARM_WHITE\n  supporte'
2021-08-25 21:53:44 INFO (MainThread) [zigpy_cc.uart] Bytes received: b'd_color_modes: COLOR_MODE_RGB\n  '
2021-08-25 21:53:44 INFO (MainThread) [zigpy_cc.uart] Bytes received: b'legacy_supports_brightness: YES\n'
2021-08-25 21:53:44 INFO (MainThread) [zigpy_cc.uart] Bytes received: b'  legacy_supports_rgb: YES\n  leg'
2021-08-25 21:53:44 INFO (MainThread) [zigpy_cc.uart] Bytes received: b'acy_supports_white_value: YES\n  '
2021-08-25 21:53:44 INFO (MainThread) [zigpy_cc.uart] Bytes received: b'legacy_supports_color_temperatur'
2021-08-25 21:53:44 INFO (MainThread) [zigpy_cc.uart] Bytes received: b'e: YES\n  min_mireds: 333.333\n  m'
2021-08-25 21:53:44 INFO (MainThread) [zigpy_cc.uart] Bytes received: b'ax_mireds: 666.667\n  disa\r\n'

Additional information

No response

[probot-home-assistant]

Hey there @dmulcahey, @Adminiuga, mind taking a look at this issue as it has been labeled with an integration (zha) you are listed as a code owner for? Thanks!
_{^{(message by CodeOwnersMention)}}

---

zha documentation
zha source
_{^{(message by IssueLinks)}}

[frenck]

Besides the added USB discovery, it is this PR #55128 that actually does the probing.
IMHO, we should never probe a serial port, unless the user manually actually selects/choose the port to be probed.

[oxan]

Besides the added USB discovery, it is this PR #55128 that actually does the probing.

That one just changes what's done with the result of the probing, probing on automatically detected devices (along with the too broad VID/PID filter) was added in #54935.

IMHO, we should never probe a serial port, unless the user manually actually selects/choose the port to be probed.

Agreed.

[frenck]

Well, the VID/PID I can imaging still... and it should not interfere.

"We might have found this..."

Maybe too broad, yeah... but can be ignored.
It's like discovering UPnP stuff, or all the HomeKit stuff ;)

I guess I try to say: The broadness is debatable... the automatic probing inexcusable.

[Adminiuga]

yeah, we should not try to probe, unless we know that adapter is a Zigbee adapter, which implies the automatic USB discovery based just on VID/PID is not enough on its own, if using a generic USB-to-Serial bridges, and must take into account something like "description" if it allows unique identification.
In other words it could work for HUSBZB-1, ConBee 2 (ConBee I?), but for things that use CH340 or default settings for CP210x, then we should not probe it

[dmulcahey]

Just peeked at this. I think the issue is specifically here:

core/homeassistant/components/zha/config_flow.py

Line 133 in 06a30c8

self._auto_detected_data = await detect_radios(dev_path)

and only affects usb discovery.

Probing has been in ZHA for quite a while and was only ever done on user selected devices. Does this make sense? Maybe we just need a tweak to USB discovery to do the probing after user confirmation.

This may have to be reworked to allow users to pick from detected usb devices rather than trying to do the work for them because of how non specific the ids are for the devices.

[bdraco]

We are probably going to have to whitelist known devices and skip the probe.

[Adminiuga]

that could be middle ground:

• Upsides:
  • Doesn't probe the port
• Downsides:
  • Will proposed ports for ZHA configuration which are not actually ZHA
  • Essentially relies on user to know whether they have a ZHA adapter or not :)

[bdraco]

Ideally we could still probe only after the started event but only if nothing else has the device open since anything that uses it should have opened it during startup. I don't think we have a good way to detect that.

[Adminiuga]

yeah, no good way to detect that. unless you run lsof and check if anything else has the port opened?
For me, just hardcoding pid/vid + description to a Zigpy radio lib would suffice :)

[bdraco]

I think we should modify the usb integration to pre-filter them out so zha only sees the ones we know are good matches.

I'll work on a PR

[oxan]

I think we should modify the usb integration to pre-filter them out

Which ones are you going to filter out though? The two I mentioned aren't the only common serial-to-USB chips, and every once in a while new ones spring up. I think it'd be better to explicitly define devices that will be probed and ignore everything else (i.e. positive identification instead of negative identification).

[bdraco]

I think we should modify the usb integration to pre-filter them out

Which ones are you going to filter out though? The two I mentioned aren't the only common serial-to-USB chips, and every once in a while new ones spring up. I think it'd be better to explicitly define devices that will be probed and ignore everything else (i.e. positive identification instead of negative identification).

whitelisting per my comment above.

[bdraco]

Unfortunately it looks like the we end up dropping discovery support for the Eletrolama zig-a-zig-ah since it uses a generic description

2021-08-25 17:04:23 DEBUG (MainThread) [homeassistant.components.usb] Discovered USB Device: USBDevice(device='/dev/ttyUSB0', vid='1A86', pid='7523', serial_number=None, manufacturer=None, description='USB2.0-Serial')

Everything else should be ok though

2021-08-25 17:02:34 DEBUG (MainThread) [homeassistant.components.usb] Discovered USB Device: USBDevice(device='/dev/ttyUSB1', vid='10C4', pid='8A2A', serial_number='612020FD', manufacturer='Silicon Labs', description='HubZ Smart Home Controller - HubZ ZigBee Com Port')
2021-08-25 17:05:00 DEBUG (MainThread) [homeassistant.components.usb] Discovered USB Device: USBDevice(device='/dev/ttyUSB0', vid='10C4', pid='EA60', serial_number='00_12_4B_00_22_98_88_7F', manufacturer='Silicon Labs', description="slae.sh cc2652rb stick - slaesh's iot stuff - slae.sh cc2652rb stick - slaesh's iot stuff")
2021-08-25 17:06:55 DEBUG (MainThread) [homeassistant.components.usb] Discovered USB Device: USBDevice(device='/dev/ttyACM0', vid='1CF1', pid='0030', serial_number='DE2433880', manufacturer='dresden elektronik ingenieurtechnik GmbH', description='ConBee II')

[Adminiuga]

Unfortunately it looks like the we end up dropping discovery support for the Eletrolama zig-a-zig-ah since it uses a generic description

Yep, Kind of expected that.

[bdraco]

I've got an implementation worked out. Its going to take a bit to test it properly though