Skip to content

fix: source get_device_in from Device/{contract} endpoint (reCAPTCHA-gated DeviceIn) - #269

Closed
Minitour wants to merge 1 commit into
GuyKh:mainfrom
Minitour:fix/device-in-recaptcha
Closed

fix: source get_device_in from Device/{contract} endpoint (reCAPTCHA-gated DeviceIn)#269
Minitour wants to merge 1 commit into
GuyKh:mainfrom
Minitour:fix/device-in-recaptcha

Conversation

@Minitour

@Minitour Minitour commented Jul 4, 2026

Copy link
Copy Markdown

Summary

get_device_in calls GET /api/DeviceIn/{contract_id}, which now returns HTTP 400 {"Error":"Token should be provide","Code":400} unless a reCAPTCHA token is supplied via a RecaptchToken header. That header can't be produced by a headless client, which breaks device/meter enumeration for downstream consumers (e.g. the Home Assistant integration — see GuyKh/iec-custom-component#462).

This PR re-sources the same device list from the reCAPTCHA-free GET /api/Device/{contract_id} endpoint and adapts it to DeviceInResponse, so the public API (IecClient.get_device_in() / data.get_device_in()) keeps the exact same return shape. No caller changes are required.

Details

  • data.get_device_in now delegates to get_devices (Device/{contract_id}) and maps each Device into a DeviceInDevice.
  • Device/{contract_id} returns the same fields as the old endpoint (isActive, deviceType, deviceNumber, deviceCode) except meterKind, so meter_kind defaults to "Consumption" (the value the RemoteReadingRange request already defaults to via SmartMeter).
  • DeviceInDevice / DeviceInResponse fields are given defaults so the models stay backward-compatible with the legacy DeviceIn payload (existing from_dict parsing still works).
  • GET_DEVICE_IN_URL is kept but annotated as deprecated.

Verification

Confirmed live against a real account:

  • GET /api/DeviceIn/{contract}400 "Token should be provide"
  • GET /api/Device/{contract}200 [{"isActive":true,"deviceType":3,"deviceNumber":"...","deviceCode":"503", ...}]

End-to-end, data.get_device_in(session, token, contract_id) now returns a populated DeviceInResponse (status=0, is_active=True, one device with meter_kind="Consumption"), and the Home Assistant integration fetches devices successfully.

Test plan

  • get_device_in returns devices for a contract with smart meter(s)
  • Downstream remote-reading calls still work using the returned device_number / device_code
  • Legacy DeviceInResponse.from_dict(...) still parses the old payload

Fixes GuyKh/iec-custom-component#462

The DeviceIn/{contract_id} endpoint now returns HTTP 400 ("Token should be
provide") unless a reCAPTCHA token is supplied via the RecaptchToken header,
which isn't feasible for headless clients (GuyKh/iec-custom-component#462).

get_device_in now sources the device list from the reCAPTCHA-free
Device/{contract_id} endpoint and adapts it to DeviceInResponse, keeping the
same return shape (meter_kind defaults to "Consumption"). DeviceInResponse and
DeviceInDevice fields are defaulted so they remain backward-compatible with the
legacy DeviceIn payload.

Co-authored-by: Cursor <cursoragent@cursor.com>
@qodo-code-review

Copy link
Copy Markdown
Contributor

PR Summary by Qodo

Fix get_device_in by sourcing devices from reCAPTCHA-free Device endpoint

🐞 Bug fix 🕐 20-40 Minutes

Grey Divider

AI Description

• Avoid broken DeviceIn/{contract} calls now gated by reCAPTCHA header requirements.
• Fetch devices via Device/{contract} and adapt results to legacy DeviceInResponse shape.
• Add safe defaults to DeviceIn models to keep backward-compatible parsing and callers.
Diagram

graph TD
  A["Client code"] --> B["data.get_device_in"] --> C["data.get_devices"] --> D[["IEC API: GET /api/Device/{contract}"]] --> E["Map to DeviceInDevice"] --> F["Return DeviceInResponse"]
  B -. "deprecated" .-> G[["IEC API: GET /api/DeviceIn/{contract}"]]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Dual-path: try DeviceIn w/ optional RecaptchToken, fallback to Device
  • ➕ Would preserve meterKind fidelity when callers can provide the token
  • ➕ Provides resilience if the Device endpoint diverges or is later gated
  • ➖ Adds branching behavior and more complicated error handling
  • ➖ Still unusable for headless clients without token generation
  • ➖ More surface area to test (multiple endpoints, precedence rules)
2. Expose a new API method returning Device model (and deprecate get_device_in)
  • ➕ Avoids maintaining a legacy response shape long-term
  • ➕ Clarifies which endpoint is authoritative
  • ➖ Breaking change or requires a long deprecation cycle
  • ➖ Forces downstream consumers (e.g., Home Assistant) to update immediately

Recommendation: Current approach (source from Device/{contract} and adapt to DeviceInResponse) is the best immediate fix because it restores headless compatibility without changing the public return shape. Consider the dual-path fallback only if meterKind becomes materially important again or if endpoint stability becomes a concern.

Files changed (3) +54 / -19

Bug fix (2) +51 / -19
data.pyRe-implement get_device_in via Device endpoint and model mapping +28/-7

Re-implement get_device_in via Device endpoint and model mapping

• Stops calling 'DeviceIn/{contract_id}' and instead calls 'get_devices' ('Device/{contract_id}'), mapping each entry into 'DeviceInDevice' while defaulting 'meter_kind' to 'Consumption'. Returns a constructed 'DeviceInResponse' preserving the legacy response shape and sets 'is_active' based on any active device.

iec_api/data.py

device_in.pyMake DeviceIn models backward-compatible with defaults and optional fields +23/-12

Make DeviceIn models backward-compatible with defaults and optional fields

• Introduces 'DEFAULT_METER_KIND = "Consumption"' and updates 'DeviceInDevice'/'DeviceInResponse' fields with defaults and optional types so both legacy 'DeviceIn' payload parsing and newly-constructed responses remain compatible.

iec_api/models/device_in.py

Documentation (1) +3 / -0
const.pyDocument DeviceIn endpoint deprecation due to reCAPTCHA gating +3/-0

Document DeviceIn endpoint deprecation due to reCAPTCHA gating

• Adds an inline deprecation note explaining that 'DeviceIn/{contract_id}' now returns HTTP 400 unless a 'RecaptchToken' header is provided, and that the code now uses 'Device/{contract_id}' instead.

iec_api/const.py

@qodo-code-review

Copy link
Copy Markdown
Contributor

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📜 Skill insights (0)

Grey Divider


Action required

1. Inactive devices returned 🐞 Bug ≡ Correctness
Description
data.get_device_in() now adapts the full /api/Device/{contract_id} list without filtering
inactive devices, even though IecClient.get_device_in() is documented as returning active devices.
Callers that select the first returned device (as in example.py) may start using an inactive meter
and fail downstream operations (e.g., remote reading).
Code

iec_api/data.py[R440-450]

+    devices = await get_devices(session, token, contract_id)
+    device_in_devices = [
+        DeviceInDevice(
+            is_active=device.is_active,
+            device_type=device.device_type,
+            device_number=device.device_number,
+            device_code=device.device_code,
+            meter_kind=DEFAULT_METER_KIND,
+        )
+        for device in (devices or [])
+    ]
Relevance

⭐⭐ Medium

No history about filtering inactive devices; get_device_in behavior recently changed for
compatibility (PR239).

PR-#239
PR-#210

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The new implementation builds DeviceInResponse directly from get_devices() without filtering;
meanwhile the client method is documented as “active devices”, and the shipped example code selects
the first device without checking activity, making ordering/contents a functional compatibility
concern.

iec_api/data.py[427-455]
iec_api/iec_client.py[670-678]
example.py[70-85]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`data.get_device_in()` returns all devices from `get_devices()` (including inactive). This conflicts with the documented expectation that `get_device_in()` provides “active devices”, and can break callers that choose the first device.

### Issue Context
- `IecClient.get_device_in()` states it returns active devices.
- `example.py` selects `device_in.devices[0]` without checking `is_active`.

### Fix Focus Areas
- Filter returned devices to active only **or** sort active devices first (preserving the old behavior expectation).
- If you choose to keep inactive devices in the response for completeness, ensure the ordering (or documentation) prevents common “pick first” callers from selecting an inactive meter.

- iec_api/data.py[427-455]
- example.py[70-85]
- iec_api/iec_client.py[670-678]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Informational

2. Active defaults mask payload 🐞 Bug ☼ Reliability
Description
DeviceInDevice.is_active and DeviceInResponse.is_active now default to True, so
DeviceInResponse.from_dict(...) can silently treat missing isActive fields as active instead of
failing fast. This can mask upstream schema regressions and lead to incorrect “active” reporting
when deserializing legacy/third-party payloads.
Code

iec_api/models/device_in.py[R38-51]

+    is_active: bool = field(default=True, metadata=field_options(alias="isActive"))
+    device_type: Optional[int] = field(default=None, metadata=field_options(alias="deviceType"))
+    device_number: Optional[str] = field(default=None, metadata=field_options(alias="deviceNumber"))
+    device_code: Optional[str] = field(default=None, metadata=field_options(alias="deviceCode"))
+    meter_kind: str = field(default=DEFAULT_METER_KIND, metadata=field_options(alias="meterKind"))


@dataclass
class DeviceInResponse(DataClassDictMixin):
-    """DeviceIn endpoint response."""
+    """Device list response (compatible with the legacy DeviceIn payload)."""

-    status: int
-    is_active: bool = field(metadata=field_options(alias="isActive"))
-    devices: list[DeviceInDevice]
+    status: int = 0
+    is_active: bool = field(default=True, metadata=field_options(alias="isActive"))
+    devices: list[DeviceInDevice] = field(default_factory=list)
Relevance

⭐ Low

Repo favors optional/default fields for missing API data (PR239); no evidence they require fail-fast
missing isActive.

PR-#239
PR-#210

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The model now encodes “active” as the fallback when fields are absent, which is a silent behavior
change in deserialization. This is similar to a previously accepted issue pattern where missing data
should not be silently converted into a valid-looking value.

iec_api/models/device_in.py[34-51]
PR-#255

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The `DeviceIn*` dataclasses now default `is_active=True`. When deserializing via `from_dict`, missing `isActive` fields will become `True` silently, which is a risky default and can hide upstream API/schema issues.

### Issue Context
Even though `data.get_device_in()` now constructs these objects explicitly, `DeviceInResponse.from_dict(...)` is still part of the public model API and may be used by callers parsing legacy payloads.

### Fix Focus Areas
- Consider defaulting `is_active` to `False` (safer) at both the response and device levels.
- Optionally add a `__post_deserialize__` validation that raises if required identifiers (e.g., `deviceNumber`, `deviceCode`) are missing when `isActive` is true.

- iec_api/models/device_in.py[34-51]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Comment thread iec_api/data.py
Comment on lines +440 to +450
devices = await get_devices(session, token, contract_id)
device_in_devices = [
DeviceInDevice(
is_active=device.is_active,
device_type=device.device_type,
device_number=device.device_number,
device_code=device.device_code,
meter_kind=DEFAULT_METER_KIND,
)
for device in (devices or [])
]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

1. Inactive devices returned 🐞 Bug ≡ Correctness

data.get_device_in() now adapts the full /api/Device/{contract_id} list without filtering
inactive devices, even though IecClient.get_device_in() is documented as returning active devices.
Callers that select the first returned device (as in example.py) may start using an inactive meter
and fail downstream operations (e.g., remote reading).
Agent Prompt
### Issue description
`data.get_device_in()` returns all devices from `get_devices()` (including inactive). This conflicts with the documented expectation that `get_device_in()` provides “active devices”, and can break callers that choose the first device.

### Issue Context
- `IecClient.get_device_in()` states it returns active devices.
- `example.py` selects `device_in.devices[0]` without checking `is_active`.

### Fix Focus Areas
- Filter returned devices to active only **or** sort active devices first (preserving the old behavior expectation).
- If you choose to keep inactive devices in the response for completeness, ensure the ordering (or documentation) prevents common “pick first” callers from selecting an inactive meter.

- iec_api/data.py[427-455]
- example.py[70-85]
- iec_api/iec_client.py[670-678]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

@Minitour

Minitour commented Jul 5, 2026

Copy link
Copy Markdown
Author

@GuyKh I didn't find your comment. I only see the qodo bot.

@GuyKh

GuyKh commented Jul 5, 2026

Copy link
Copy Markdown
Owner

@GuyKh I didn't find your comment. I only see the qodo bot.

My comment was about async def get_device_in(session: ClientSession, token: JWT, contract_id: str) -> Optional[DeviceInResponse]: change.

It obfuscates the actual API behavior; And transforms one object to the other, just to preserve the library API.
The idea for this library is to expose the IEC API directly, so if it now accepts a new header, it should be reflected in the API.
Changes in the usage, should be changed on the using side

@Minitour

Minitour commented Jul 5, 2026

Copy link
Copy Markdown
Author

Makes sense. Until we meet again, cheers 🍻

@Minitour Minitour closed this Jul 5, 2026
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

Successfully merging this pull request may close these issues.

Addon stopped working a few days ago - bad call to DeviceIn

2 participants