zigpy / zigpy-znp

TI CC2531, CC13x2, CC26x2 radio support for Zigpy and ZHA
GNU General Public License v3.0
145 stars 40 forks source link

What happens if user set tx_power to 20 for CC2652P/CC1352P instead of 19 which looks to be current max? #125

Closed Hedda closed 2 years ago

Hedda commented 2 years ago

I think a real-world could problem could be that many ZHA users will just assume that 20 will be the tx_power maximum that they can set even if the documentation says the range only goes to 19 because both Texas Instruments and Zigbee coordinator adapter manufacturers (such as ITead/Sonoff) are marketing CC2652P/CC1352P dongles as having "+20 dBm" power amplification.

https://github.com/zigpy/zigpy-znp/blob/dev/README.md#configuration

      # Only if your stick has a built-in power amplifier (i.e. CC1352P and CC2592)
      # If set, must be between:
      #  * CC1352/2652:  -22 and 19
      #  * CC253x:       -22 and 22
      tx_power:  

For consistency might it not be a good idea to allow a tx_power maximum of 20 in zigpy-znp/ZHA configuration?

Might otherwise also confuse users coming to ZHA from Zigbee2MQTT/Z2M as it allows users to set transmit_power to 20:

https://www.zigbee2mqtt.io/guide/configuration/adapter-settings.html#transmitter-power

experimental:
  # Optional: Transmit power setting in dBm (default: 5).
  # This will set the transmit power for devices that bring an inbuilt amplifier.
  # It can't go over the maximum of the respective hardware and might be limited
  # by firmware (for example to migrate heat, or by using an unsupported firmware).
  # For the CC2652R(B) this is 5 dBm, CC2652P/CC1352P-2 20 dBm.
  transmit_power: 5

@puddly What happens if ZHA users set tx_power to 20 for CC2652P/CC1352P instead of 19 which look to be the current max?

As I understand, the CC2538 + CC2592 TX Power maximum was 19, but the TX Power max for newer CC2652P/CC1352P is 20?

Did you internally change tx_power maximum to allow 20 for CC2652P/CC1352P in zigpy-znp or is 19 still max for all types?

At least think when you implemented tx_power setting in zigpy-znp it was for CC2538 + CC2592 only which has max tx_power 19?

https://github.com/zigpy/zigpy-znp/issues/81

mdeweerd commented 2 years ago

The CC2530 ZNP stack compares the requested value to the maximum in the internal table and caps the value to the maximum. I suppose this is the same for the newer stacks.

Hedda commented 2 years ago

The CC2530 ZNP stack compares the requested value to the maximum in the internal table and caps the value to the maximum. I suppose this is the same for the newer stacks.

Do you mean it is already possible to set tx_power to 20 for zigpy-znp (in HA's ZHA) so it will accept and pass to ZNP stack which internally will use 20? Or will zigpy-znp reject setting if try to configure tx_power to 20 instead of 19 with is mentioned as max?

Again, both zigpy-znp docs currently state that 19 is highest can set tx_power for CC1352/2652 (i.e. CC2652P/CC1352P):

https://github.com/zigpy/zigpy-znp/blob/dev/README.md#configuration

      # Only if your stick has a built-in power amplifier (i.e. CC1352P and CC2592)
      # If set, must be between:
      #  * CC1352/2652:  -22 and 19
      #  * CC253x:       -22 and 22
      tx_power:  

ZHA docs does not yet mention tx_power:

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

mdeweerd commented 2 years ago

Do you mean it is already possible to set tx_power to 20 for zigpy-znp (in HA's ZHA) so it will accept and pass to ZNP stack which internally will use 20? Or will zigpy-znp reject setting if try to configure tx_power to 20 instead of 19 with is mentioned as max?

I do not know if zigpy-znp sets a limit, but I referred to the ZNP firmware itself. The protocol has a "set_tx_power" command and if you exceed the value that is configured in the firmware it is capped to the maximum. So if you ask for 20 and the firmware is set to 5 then it will be "5" . The firmware is hard coded with the limits (which depend on all hardware involved, including a potential RF amplifier).

puddly commented 2 years ago

The set_tx_power option forwards the value directly to the firmware via c.SYS.SetTxPower.Req(TXPower=dbm).

I've confirmed that (reverse) LQI steadily increases as TX power increases and plateaus after reaching the firmware maximum. Setting it to 0, unplugging the coordinator, then to 100, unplugging the coordinator, and then letting it run does not cause the LQI to decrease, confirming that the firmware limits it.