KeyError on OmadaListDevice.need_upgrade and OmadaListDevice.fw_download

odouville commented 1 year ago

(Seen on version 1.2.4)

When retrieving status on devices, I sometimes get the error KeyError: 'needUpgrade' or KeyError: 'fwDownload' (see tracebacks below). It happens when, for example, the device is disconnected (seen on a spare AP that I keep in a closet).

The following code is used:

    for device in (await site_client.get_devices()):
        print(device.raw_data)
        print(f'Device {device.name}: Firmware downloading: {device.fw_download}')

(same code with need_upgrade instead of fw_download can be used as well)

Traceback for need_upgrade:

Traceback (most recent call last):
  File "C:\Users\***\PycharmProjects\OmadaClient\main.py", line 54, in <module>
    asyncio.run(test_omada())
  File "C:\Users\***\AppData\Local\Programs\Python\Python311\Lib\asyncio\runners.py", line 190, in run
    return runner.run(main)
           ^^^^^^^^^^^^^^^^
  File "C:\Users\***\AppData\Local\Programs\Python\Python311\Lib\asyncio\runners.py", line 118, in run
    return self._loop.run_until_complete(task)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\***\AppData\Local\Programs\Python\Python311\Lib\asyncio\base_events.py", line 650, in run_until_complete
    return future.result()
           ^^^^^^^^^^^^^^^
  File "C:\Users\***\PycharmProjects\OmadaClient\main.py", line 35, in test_omada
    print(f'Device {device.name}: Need upgrade: {device.need_upgrade}')
                                                 ^^^^^^^^^^^^^^^^^^
  File "C:\Users\***\PycharmProjects\OmadaClient\venv\Lib\site-packages\tplink_omada_client\devices.py", line 112, in need_upgrade
    return self._data["needUpgrade"]
           ~~~~~~~~~~^^^^^^^^^^^^^^
KeyError: 'needUpgrade'

Traceback for fw_download:

Traceback (most recent call last):
  File "C:\Users\***\PycharmProjects\OmadaClient\main.py", line 54, in <module>
    asyncio.run(test_omada())
  File "C:\Users\***\AppData\Local\Programs\Python\Python311\Lib\asyncio\runners.py", line 190, in run
    return runner.run(main)
           ^^^^^^^^^^^^^^^^
  File "C:\Users\***\AppData\Local\Programs\Python\Python311\Lib\asyncio\runners.py", line 118, in run
    return self._loop.run_until_complete(task)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\***\AppData\Local\Programs\Python\Python311\Lib\asyncio\base_events.py", line 650, in run_until_complete
    return future.result()
           ^^^^^^^^^^^^^^^
  File "C:\Users\***\PycharmProjects\OmadaClient\main.py", line 35, in test_omada
    print(f'Device {device.name}: Firmware downloading: {device.fw_download}')
                                                 ^^^^^^^^^^^^^^^^^^
  File "C:\Users\***\PycharmProjects\OmadaClient\venv\Lib\site-packages\tplink_omada_client\devices.py", line 112, in fw_download
    return self._data["fwDownload"]
           ~~~~~~~~~~^^^^^^^^^^^^^^
KeyError: 'fwDownload'

odouville commented 1 year ago

Here's the raw data for a connected AP:

{
   "type":"ap",
   "mac":"MA-CA-DD-RE-SS-xx",
   "name":"EAP24502",
   "model":"EAP245",
   "compoundModel":"EAP245(EU) v3.0",
   "showModel":"EAP245(EU) v3.0",
   "modelVersion":"3.0",
   "firmwareVersion":"5.0.6 Build 20220429 Rel. 44315",
   "version":"5.0.6",
   "hwVersion":"3.0",
   "ip":"IP.ADD.RE.SS",
   "uptime":"12day(s) 20h 8m 17s",
   "uptimeLong":1109297,
   "statusCategory":1,
   "status":14,
   "adoptFailType":-1,
   "lastSeen":1682857232591,
   "needUpgrade":false,
   "fwDownload":false,
   "cpuUtil":1,
   "memUtil":50,
   "download":80275048449,
   "upload":56655231893,
   "site":"*****",
   "location":{
      "located":false
   },
   "clientNum":9,
   "compatible":0,
   "locateEnable":false,
   "sn":"*****",
   "combinedGateway":false,
   "wirelessLinked":false,
   "deviceMisc":{
      "support5g":true,
      "support5g2":false,
      "support6g":false,
      "support11ac":true,
      "supportLag":false,
      "supportMesh":1,
      "customizeRegion":276,
      "minPower2G":9,
      "maxPower2G":20,
      "minPower5G":10,
      "maxPower5G":28,
      "supportChannelLimit":false,
      "supportDfs":7,
      "supportRoaming":1
   },
   "devCap":{
      "supportPa":0,
      "meshChainNum":3,
      "supportOFDMA2g":false,
      "supportOFDMA5g":false,
      "supportOFDMA5g2":false,
      "supportOFDMA6g":false,
      "supportMeshPriority":true,
      "supportL3Access":false
   },
   "wlanGroup":"Default + Guest",
   "override":"0/7",
   "bssids":[
      "MA-CA-DD-RE-SS-xx",
      "MA-CA-DD-RE-SS-xx",
      "MA-CA-DD-RE-SS-xx"
   ],
   "radioSetting2g":{
      "radioEnable":true,
      "channelWidth":"4",
      "channel":"0",
      "txPower":20,
      "txPowerLevel":2
   },
   "radioSetting5g":{
      "radioEnable":true,
      "channelWidth":"6",
      "channel":"0",
      "txPower":28,
      "txPowerLevel":2
   },
   "wp2g":{
      "actualChannel":"6   / 2437MHz",
      "maxTxRate":450,
      "txPower":20,
      "region":276,
      "bandWidth":"Auto",
      "rdMode":"b/g/n mixed",
      "txUtil":4,
      "rxUtil":12,
      "interUtil":1
   },
   "wp5g":{
      "actualChannel":"36  / 5180MHz",
      "maxTxRate":1300,
      "txPower":28,
      "region":276,
      "bandWidth":"Auto",
      "rdMode":"a/n/ac mixed",
      "txUtil":0,
      "rxUtil":0,
      "interUtil":0
   },
   "txRate":11634,
   "rxRate":13696,
   "clientNum2g":7,
   "clientNum5g":2,
   "clientNum5g2":0,
   "clientNum6g":0,
   "userNum":9,
   "guestNum":0,
   "hop":0,
   "downlink":0,
   "anyPoeEnable":false,
   "licenseStatusStr":"-"
}

odouville commented 1 year ago

And the raw data for a disconnected AP:

{
   "type":"ap",
   "mac":"MA-CA-DD-RE-SS-xx",
   "name":"EAP24503",
   "model":"EAP245",
   "compoundModel":"EAP245(EU) v3.0",
   "showModel":"EAP245(EU) v3.0",
   "modelVersion":"3.0",
   "firmwareVersion":"5.0.6 Build 20220429 Rel. 44315",
   "version":"5.0.6",
   "hwVersion":"3.0",
   "ip":"IP.ADD.RE.SS",
   "statusCategory":0,
   "status":0,
   "lastSeen":1666277825817,
   "site":"*****",
   "location":{
      "located":false
   },
   "clientNum":0,
   "compatible":0,
   "sn":"*****",
   "combinedGateway":false,
   "wirelessLinked":false,
   "deviceMisc":{
      "support5g":true,
      "support5g2":false,
      "support6g":false,
      "support11ac":true,
      "supportLag":false,
      "supportMesh":1,
      "customizeRegion":276,
      "minPower2G":9,
      "maxPower2G":20,
      "minPower5G":10,
      "maxPower5G":28,
      "supportChannelLimit":false,
      "supportDfs":7,
      "supportRoaming":1
   },
   "devCap":{
      "supportPa":0,
      "meshChainNum":3,
      "supportOFDMA2g":false,
      "supportOFDMA5g":false,
      "supportOFDMA5g2":false,
      "supportOFDMA6g":false,
      "supportMeshPriority":true,
      "supportL3Access":false
   },
   "wlanGroup":"Default + Guest",
   "override":"0/7",
   "radioSetting2g":{
      "radioEnable":true,
      "channelWidth":"4",
      "channel":"0",
      "txPower":20,
      "txPowerLevel":2
   },
   "radioSetting5g":{
      "radioEnable":true,
      "channelWidth":"6",
      "channel":"0",
      "txPower":28,
      "txPowerLevel":2
   },
   "clientNum2g":0,
   "clientNum5g":0,
   "clientNum5g2":0,
   "clientNum6g":0,
   "hop":0,
   "downlink":0,
   "anyPoeEnable":false,
   "licenseStatusStr":"-"
}

odouville commented 1 year ago

Same behavior with uptimeLong, uptime, cpuUtil and memUtil properties in the JSON payload returned by the API.

MarkGodwin commented 1 year ago

This is incredibly useful information - thank you!

odouville commented 1 year ago

Indeed, but there may be other cases in which the returned payload doesn't contain all of the properties the API can expose (pending device, isolated...)

MarkGodwin commented 1 year ago

True. I might change the code to just report defaults if the key is missing instead. With the code as it is now, it may refuse to return information when it is actually present. It's fixed in v1.2.5 now, using the explicit check on statusCategory. I changed your proposed fix to return default values to make the API simpler to consume.

odouville commented 1 year ago

I too thought about using default values according to each datatype. However by doing this the fact that the data is not available is completely obfuscated. It might not be so important for properties like cpuUtil or memUtil, but I'm a little bit more concerned when it comes to the need_upgrade property or the PoE power of a switch port (another PR I'm working on). I'll try to explain my POV.

Regarding the need_upgrade status, having a default False returned could lets the user think that no upgrade is available, which is wrong (in fact the upgradability of the device cannot be checked because it is not available). Tough, considering that a boolean value cannot be combined with a NoneType (in terms of [EDIT]~~logical~~bitwise operations), maybe we have no other proper choice than returning a default False value to make, as you wrote, the API simpler to consume.

[EDIT] The above sentence seams totally meaningless when we consider logical operators, instead of bitwise operators as I was first looking at. As I'm still discovering Python, just made an additional test:

print("None is a " + ("truthy" if None else "falsy"))

And None is indeed a falsy, so it could be returned as a default value and still be used in logical expressions. That would keep the actual meaning of the False value intact (which is "The upgradability could be checked and no upgrade is available."). [END EDIT]

Regarding the missing POE power returned as the default value, it's worth thinking of it. We know that a port without any device connected will be reported as a DOWN link status on that port. Then the API consumer can rely on that piece of information to detect an unused port. But when the link status is UP:

a port reporting a 0.0 power consumption means that the port has PoE capability but no power is consumed by any device,
a port reporting no PoE power value means that the port has NO PoE capability.

The lack of PoE capability on one port can only be determined by a missing poe_power value in the Omada API output.

odouville commented 1 year ago

I created another MR for the PoE capability fix. Let's consider this issue as fixed by #17 and your following commits!

MarkGodwin / tplink-omada-api

KeyError on OmadaListDevice.need_upgrade and OmadaListDevice.fw_download #18