Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

KeyError on OmadaListDevice.need_upgrade and OmadaListDevice.fw_download #18

Closed
odouville opened this issue Apr 30, 2023 · 8 comments
Closed

Comments

@odouville
Copy link
Contributor

(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
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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

@MarkGodwin
Copy link
Owner

This is incredibly useful information - thank you!

@odouville
Copy link
Contributor Author

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
Copy link
Owner

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
Copy link
Contributor Author

odouville commented Apr 30, 2023

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]logicalbitwise 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
Copy link
Contributor Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants