Repository navigation
Sign v1r requests through EnergyDeviceSigner - #206
Conversation
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Signer delegation and request validation tesla_fleet_api/tesla/powerwall/transport.py, tests/powerwall/test_transport_signing.py |
V1rTransport initializes EnergyDeviceSigner with the loaded private key and 12-second TTL, then offloads signing to a thread. Deterministic tests check the request bytes, reply payload, message fields, expiry, and RSA signature. |
Priority:
Estimated code review effort: 2 (Simple) | ~12 minutes
Change: Refactor
Possibly related PRs
- Teslemetry/python-tesla-fleet-api#199: Introduces EnergyDeviceSigner, which PR
#206uses in V1rTransport. - Teslemetry/python-tesla-fleet-api#205: Adds V1rTransport and the local signing implementation that PR
#206replaces with signer delegation.
Merge Risk: 
3cec8
-
No merge-blocking risk is evident for the signing delegation.
Pre-merge checks: 
-
Check name Status Explanation Resolution Docstring Coverage Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… Write docstrings for the functions missing them to satisfy the coverage threshold. -
Check name Status Explanation Title check The title clearly and concisely identifies the main change: routing v1r request signing through EnergyDeviceSigner. Description check The description directly explains the signing change, preserved behavior, removed duplicate code, and added deterministic tests related to the changeset. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 2 files.
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
V1rTransport now signs /tedapi/v1r requests through tesla/energy_signing.EnergyDeviceSigner with ttl=12, called in asyncio.to_thread, instead of its own copy of the signing code; it still loads the PEM itself and raises PowerwallAuthenticationError on a bad or non-RSA key, and _tlv, _build_tlv, _sign, the TLV tag constants and the signing-only imports are deleted. EnergyDeviceSigner is unchanged and its cloud default TTL stays 30 s, and the LAN and cloud envelope builders stay separate. The first commit adds tests/powerwall/test_transport_signing.py, a fixed byte vector test (fixed RSA key, envelope, DIN, clock and message UUID) that checks the whole signed RoutableMessage posted by post_v1r and verifies the signature and the 12 s expires_at; it passed against the old _sign code, and the second commit makes the signer switch with the test file unchanged and it still passes, which proves the signed bytes are identical. No other behaviour changes.