Add REST API support with backward-compatible auto detection by espipj · Pull Request #454 · iMicknl/python-sagemcom-api

espipj · 2026-02-22T01:02:38Z

Summary

This PR adds support for newer Sagemcom firmware that exposes /api/v1/* endpoints, while preserving compatibility with legacy /cgi/json-req firmware.

What Changed

Added ApiMode enum:
- ApiMode.AUTO (default)
- ApiMode.LEGACY
- ApiMode.REST
Extended SagemcomClient with api_mode parameter.
Implemented API mode selection logic in login():
- LEGACY: force legacy flow
- REST: force REST flow
- AUTO: try legacy first, fallback to REST when legacy endpoint appears unavailable.
Added REST request path for:
- login() via POST /api/v1/login (form-encoded)
- logout() via POST /api/v1/logout
- get_device_info() via GET /api/v1/device
- get_hosts() via GET /api/v1/home
Kept legacy behavior for XPath-based methods; these now raise clear NotImplementedError in REST mode.

Backward Compatibility

Existing usage remains compatible.
Default mode is AUTO, so legacy users do not need to change code.
Explicit mode can be set when deterministic behavior is required.

Tests

Added unit test for AUTO fallback to REST on legacy 503 HTML response.
Added unit test for REST get_hosts() parsing (wireless + ethernet devices).

Documentation

README updated with:
- ApiMode usage in quickstart
- API mode behavior and REST/legacy limitations

iMicknl · 2026-04-06T21:28:46Z

Apologies for the delay, @espipj! Happy to take a better look at this soon, but I might need to first fix our CI/CD...

Copilot

Pull request overview

Adds support for newer Sagemcom firmwares exposing /api/v1/* REST endpoints while maintaining compatibility with the legacy /cgi/json-req (XPath) API, including an AUTO mode that attempts to pick the right API at runtime.

Changes:

Introduced ApiMode (AUTO/LEGACY/REST) and extended SagemcomClient to select/track the active API mode.
Implemented REST flows for login(), logout(), get_device_info(), get_hosts(), and reboot(), while guarding XPath methods as legacy-only.
Added/updated unit tests and README documentation for the new API modes and REST parsing.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

File	Description
`sagemcom_api/client.py`	Adds REST request/parse paths, API mode selection, and legacy-only guards.
`sagemcom_api/enums.py`	Adds `ApiMode` and `EncryptionMethod.NONE`.
`tests/unit/test_client_basic.py`	Adds tests for AUTO fallback to REST, REST host parsing, and REST reboot.
`README.md`	Documents API mode behavior and updates quickstart usage.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-07T06:53:20Z

sagemcom_api/client.py

+        payload = urllib.parse.urlencode(data or {})
+        request_headers = {"Content-Type": "application/x-www-form-urlencoded"}
+
+        async with self.session.request(method, url, data=payload, headers=request_headers) as response:


__rest_request() always sends a form-encoded body via data=payload, even for GET requests (e.g., /api/v1/device, /api/v1/home, /api/v1/hosts). Some servers reject or ignore GET bodies; this can break REST mode. Consider using params=data (and omit the Content-Type header) for GET/HEAD, and only send form-encoded data for methods that actually support a request body (POST/PUT/PATCH).

Suggested change

payload = urllib.parse.urlencode(data or {})

request_headers = {"Content-Type": "application/x-www-form-urlencoded"}

async with self.session.request(method, url, data=payload, headers=request_headers) as response:

request_method = method.upper()

request_kwargs: dict[str, Any] = {}

if request_method in {"GET", "HEAD"}:

if data:

request_kwargs["params"] = data

elif request_method in {"POST", "PUT", "PATCH"}:

request_kwargs["data"] = urllib.parse.urlencode(data or {})

request_kwargs["headers"] = {

"Content-Type": "application/x-www-form-urlencoded"

}

elif data:

request_kwargs["data"] = urllib.parse.urlencode(data)

request_kwargs["headers"] = {

"Content-Type": "application/x-www-form-urlencoded"

}

async with self.session.request(request_method, url, **request_kwargs) as response:

Copilot · 2026-04-07T06:53:20Z

sagemcom_api/client.py

+            result = await response.text()
+            if response.status in (401, 403):
+                raise UnauthorizedException(result)
+
+            if response.status == 404:
+                raise UnsupportedHostException(result)
+
+            if response.status == 400:
+                raise AuthenticationException(result)
+


__rest_request() maps HTTP 400 to AuthenticationException, but 400 is generally a bad request (and your own tests show /api/v1/home can return 400 for non-auth reasons). This can mislead callers and makes error handling inconsistent with the legacy path. Consider raising BadRequestException (or a REST-specific exception) for 400, and reserve AuthenticationException/UnauthorizedException for 401/403-like cases.

Copilot · 2026-04-07T06:53:21Z

sagemcom_api/client.py

+    async def get_encryption_method(self) -> EncryptionMethod:
        """Determine which encryption method to use for authentication and set it directly."""
+        if self.api_mode == ApiMode.REST:
+            return EncryptionMethod.NONE
+
+        if self.api_mode == ApiMode.AUTO and await self.__probe_rest_availability():
+            return EncryptionMethod.NONE
+


In AUTO mode, get_encryption_method() returns EncryptionMethod.NONE as soon as __probe_rest_availability() succeeds. That probe only proves REST login works, not that the legacy JSON-REQ API is unavailable; on dual-stack firmware this would incorrectly select NONE, and EncryptionMethod.NONE is not compatible with legacy hashing (it results in sending the raw password). Consider removing this early return, or only returning NONE when legacy probing fails with the same conditions used by __should_fallback_to_rest() (i.e., legacy endpoint appears unavailable).

Copilot · 2026-04-07T06:53:21Z

sagemcom_api/client.py

+            device = data[0].get("device", {})
+            return DeviceInfo(
+                mac_address=device.get("wan_mac_address"),
+                serial_number=device.get("serialnumber"),
+                model_name=device.get("modelname"),
+                model_number=device.get("modelname"),
+                product_class=device.get("modelname"),
+                software_version=device.get("running", {}).get("version"),


REST get_device_info() assumes data[0] is a dict and calls .get(...) unconditionally. If the endpoint returns an empty list or a list with a non-dict element, this will raise AttributeError instead of the intended UnknownException. Add a type check for data[0] (and for the nested device object) before accessing .get().

Suggested change

device = data[0].get("device", {})

return DeviceInfo(

mac_address=device.get("wan_mac_address"),

serial_number=device.get("serialnumber"),

model_name=device.get("modelname"),

model_number=device.get("modelname"),

product_class=device.get("modelname"),

software_version=device.get("running", {}).get("version"),

first_item = data[0]

if not isinstance(first_item, dict):

raise UnknownException("Invalid response from /api/v1/device")

device = first_item.get("device", {})

if not isinstance(device, dict):

raise UnknownException("Invalid response from /api/v1/device")

running = device.get("running", {})

if not isinstance(running, dict):

running = {}

return DeviceInfo(

mac_address=device.get("wan_mac_address"),

serial_number=device.get("serialnumber"),

model_name=device.get("modelname"),

model_number=device.get("modelname"),

product_class=device.get("modelname"),

software_version=running.get("version"),

Copilot · 2026-04-07T06:53:21Z

tests/unit/test_client_basic.py

+@pytest.mark.asyncio
+async def test_default_session_accepts_ip_cookies():
+    """Default aiohttp session should accept cookies from IP hosts."""
+    client = SagemcomClient(
+        host="192.168.1.1",
+        username="admin",
+        password="admin",
+        authentication_method=EncryptionMethod.MD5,
+    )
+    try:
+        assert getattr(client.session.cookie_jar, "_unsafe", False) is True
+    finally:
+        await client.close()


This test asserts client.session.cookie_jar._unsafe (a private aiohttp attribute). That makes the test brittle across aiohttp versions. Prefer asserting behavior instead (e.g., set a cookie for an IP-based URL via cookie_jar.update_cookies(...) and assert it is retained), or mock ClientSession creation and assert CookieJar(unsafe=True) was passed.

espipj requested a review from iMicknl as a code owner February 22, 2026 01:02

espipj mentioned this pull request Feb 23, 2026

For router Sagecom Fast 5598 (CS 39000) adding integration failing iMicknl/ha-sagemcom-fast#237

Open

espipj and others added 13 commits April 6, 2026 22:46

Add API mode support with legacy/rest auto-detection

4853c76

Add unit tests for REST mode and auto fallback

ca19a9d

Document API mode usage and REST limitations

9d0cead

Add REST reboot support and hosts endpoint fallback

cf65aaf

Accept REST cookies from IP hosts with unsafe CookieJar

992df7d

Add test for IP-cookie acceptance in default session

6263e08

Expose EncryptionMethod.NONE results

38fb6d6

Log API mode outcome and logout response

970d314

Make authentication_method optional again

8972255

Remove client logging instrumentation

5b261e0

Probe REST when auto detecting encryption

2b9cbac

Document YouFibre 5598 entry

a88cd19

Fix post-rebase prek issues

833f12b

iMicknl force-pushed the feat/new-API-support branch from 48dbdfd to 833f12b Compare April 7, 2026 06:44

iMicknl requested a review from Copilot April 7, 2026 06:48

Copilot started reviewing on behalf of iMicknl April 7, 2026 06:49 View session

Copilot AI reviewed Apr 7, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add REST API support with backward-compatible auto detection#454

Add REST API support with backward-compatible auto detection#454
espipj wants to merge 13 commits intoiMicknl:mainfrom
espipj:feat/new-API-support

espipj commented Feb 22, 2026

Uh oh!

iMicknl commented Apr 6, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 7, 2026

Uh oh!

Copilot AI Apr 7, 2026

Uh oh!

Copilot AI Apr 7, 2026

Uh oh!

Copilot AI Apr 7, 2026

Uh oh!

Copilot AI Apr 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

-        payload = urllib.parse.urlencode(data or {})
-        request_headers = {"Content-Type": "application/x-www-form-urlencoded"}
-        async with self.session.request(method, url, data=payload, headers=request_headers) as response:
+        request_method = method.upper()
+        request_kwargs: dict[str, Any] = {}
+        if request_method in {"GET", "HEAD"}:
+            if data:
+                request_kwargs["params"] = data
+        elif request_method in {"POST", "PUT", "PATCH"}:
+            request_kwargs["data"] = urllib.parse.urlencode(data or {})
+            request_kwargs["headers"] = {
+                "Content-Type": "application/x-www-form-urlencoded"
+            }
+        elif data:
+            request_kwargs["data"] = urllib.parse.urlencode(data)
+            request_kwargs["headers"] = {
+                "Content-Type": "application/x-www-form-urlencoded"
+            }
+        async with self.session.request(request_method, url, **request_kwargs) as response:

Uh oh!

Conversation

espipj commented Feb 22, 2026

Summary

What Changed

Backward Compatibility

Tests

Documentation

Uh oh!

iMicknl commented Apr 6, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants