Add basic controller firmware update information#72
Conversation
There was a problem hiding this comment.
Pull request overview
Adds controller-level firmware update capabilities to the Omada client library and exposes firmware status via a new CLI command.
Changes:
- Add controller API methods to check for available firmware updates, trigger an upgrade, and query upgrade status.
- Introduce new data wrappers for controller hardware update info and upgrade status.
- Add a
firmwareCLI command to display controller + device firmware versions/update availability, and bump package version to 1.5.5.
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| uv.lock | Bumps locked package version to 1.5.5. |
| pyproject.toml | Bumps project version to 1.5.5. |
| src/tplink_omada_client/omadaclient.py | Adds controller firmware update/check/status methods. |
| src/tplink_omada_client/definitions.py | Adds new API data wrappers for controller update info + upgrade status. |
| src/tplink_omada_client/devices.py | Makes device model/latest firmware access more defensive. |
| src/tplink_omada_client/cli/command_firmware.py | New CLI command to show firmware status and (optionally) release notes. |
| src/tplink_omada_client/cli/init.py | Registers the new firmware subcommand. |
| src/tplink_omada_client/init.py | Exports newly added public types. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| firmware = await site_client.get_firmware_details(device) | ||
| status = "\u2757 UPDATE" if firmware.current_version != firmware.latest_version else "\u2713 UP-TO-DATE" | ||
| print(f"{device.name:<30} {firmware.current_version:<36} {firmware.latest_version:<36} {status}") | ||
| if firmware.current_version != firmware.latest_version and firmware.release_notes and args["release_notes"]: | ||
| print(f" Release Notes: {firmware.release_notes}") |
There was a problem hiding this comment.
firmware.release_notes can raise KeyError because OmadaFirmwareUpdate.release_notes indexes self._data["fwReleaseLog"] directly. Since this command accesses firmware.release_notes, it can crash when the API omits that field. Consider using firmware.raw_data.get("fwReleaseLog") here, or updating OmadaFirmwareUpdate.release_notes to return str | None via .get() and then checking for None.
| def arg_parser(subparsers) -> None: | ||
| """Configures arguments parser for 'firmware' command""" | ||
| firmware_parser: ArgumentParser = subparsers.add_parser( | ||
| "firmware", help="Shows firmware information and update availability for the specified device" |
There was a problem hiding this comment.
Help text says this command shows firmware info for the “specified device”, but the implementation prints controller firmware plus firmware status for all devices in the configured site. Update the help string to reflect what the command actually does to avoid confusing CLI users.
| "firmware", help="Shows firmware information and update availability for the specified device" | |
| "firmware", help="Shows controller and device firmware information and update availability for the configured site" |
|
|
||
| @property | ||
| def reboot_time(self) -> int: | ||
| """How long after the donwload completes before we expect the controller to come back online, in seconds.""" |
There was a problem hiding this comment.
Typo in docstring: “donwload” should be “download”.
| """How long after the donwload completes before we expect the controller to come back online, in seconds.""" | |
| """How long after the download completes before we expect the controller to come back online, in seconds.""" |
| async def upgrade_controller_firmware(self, target_version: str) -> bool: | ||
| """Upgrade the Omada hardware controller firmware to the specified version.""" | ||
| url = self._api.format_url("cmd/upgradeFirmware") | ||
| payload = {"targetVersion": target_version} | ||
| await self._api.request("post", url, json=payload) | ||
| return True |
There was a problem hiding this comment.
upgrade_controller_firmware returns True unconditionally, even though the request can fail/raise. This makes the return value misleading and redundant. Consider returning None (let exceptions signal failure) or returning/deriving a meaningful value from the API response (e.g., accepted/started status).
| async def upgrade_controller_firmware(self, target_version: str) -> bool: | |
| """Upgrade the Omada hardware controller firmware to the specified version.""" | |
| url = self._api.format_url("cmd/upgradeFirmware") | |
| payload = {"targetVersion": target_version} | |
| await self._api.request("post", url, json=payload) | |
| return True | |
| async def upgrade_controller_firmware(self, target_version: str) -> None: | |
| """Upgrade the Omada hardware controller firmware to the specified version.""" | |
| url = self._api.format_url("cmd/upgradeFirmware") | |
| payload = {"targetVersion": target_version} | |
| await self._api.request("post", url, json=payload) |
| async def check_firmware_updates(self) -> OmadaControllerUpdateInfo: | ||
| """Check if firmware updates are available for the Omada hardware controller | ||
|
|
||
| Software Controller updates are probably not available via this API | ||
| """ | ||
| result = await self._api.request("get", self._api.format_url("controller/notification/updateInfo")) | ||
|
|
||
| return OmadaControllerUpdateInfo(result) |
There was a problem hiding this comment.
The PR description says check_firmware_updates “returns information … only if there is an update”, but the implementation always returns an OmadaControllerUpdateInfo instance. Either update the method contract (e.g., return OmadaControllerUpdateInfo | None and return None when there are no updates), or adjust the description/docstring to match the actual behavior.
New functions at the controller level -