Address Home Assistant Core review findings #17

devinslick · 2026-02-01T13:44:15Z

Key changes:

Add decrypt_data_blob_async() to prevent event loop blocking
Export Location, PhotoResult, RateLimitError from package root
Remove dead code (_parse_location_blob function)
Remove unused get_history start/end parameters
Fix lock message sanitization to re-collapse spaces after removing chars
Add docstrings to helper functions
Remove placeholder comment from helpers.py

Test improvements:

Add test for async decryption method
Add test for empty sanitized lock message
Update test for removed _parse_location_blob
Achieve 100% branch coverage

Key changes: - Add decrypt_data_blob_async() to prevent event loop blocking - Export Location, PhotoResult, RateLimitError from package root - Remove dead code (_parse_location_blob function) - Remove unused get_history start/end parameters - Fix lock message sanitization to re-collapse spaces after removing chars - Add docstrings to helper functions - Remove placeholder comment from helpers.py Test improvements: - Add test for async decryption method - Add test for empty sanitized lock message - Update test for removed _parse_location_blob - Achieve 100% branch coverage https://claude.ai/code/session_019KoQsx1g9tLyTsu2JakAnn

- Add CHANGELOG.md following Keep a Changelog format (v2.0.0 to current) - Document Raises sections for all public API methods in client.py - Update HOME_ASSISTANT_REVIEW.md to mark completed items https://claude.ai/code/session_019KoQsx1g9tLyTsu2JakAnn

Release includes: - decrypt_data_blob_async() for non-blocking decryption - Location, PhotoResult, RateLimitError exported from package root - CHANGELOG.md and comprehensive exception documentation - Lock message sanitization fix - Removed dead code and unused parameters No breaking changes for existing integrations. https://claude.ai/code/session_019KoQsx1g9tLyTsu2JakAnn

codecov · 2026-02-01T13:45:10Z

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Copilot

Pull request overview

This PR addresses items from the Home Assistant Core review by adding an async decryption path, tightening the public API surface, improving docs, and increasing test coverage.

Changes:

Added FmdClient.decrypt_data_blob_async() plus tests to enable non-blocking decryption in async environments and updated documentation to clarify sync vs async usage.
Simplified Device.get_history() (removing unused time-window parameters), improved lock-message sanitization semantics, and expanded package exports (Location, PhotoResult, RateLimitError).
Introduced a CHANGELOG.md, expanded docstrings and Home Assistant review documentation, and bumped the library version to 2.0.8 with corresponding tests to maintain high coverage.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
`tests/unit/test_lock_message.py`	Adds a regression test ensuring that messages composed only of stripped characters correctly fall back to a plain `lock` command, validating the updated sanitization logic.
`tests/unit/test_coverage_improvements.py`	Replaces the `_parse_location_blob` test with a new test for `decrypt_data_blob_async()` and adds coverage for picture retrieval behavior, contributing to the stated coverage goals.
`pyproject.toml`	Bumps the project version to `2.0.8` and keeps tooling/coverage configuration aligned with the new release.
`fmd_api/helpers.py`	Refines the module and function docstrings for base64 padding/decoding helpers, making their behavior clearer while leaving logic unchanged.
`fmd_api/device.py`	Removes the unused `_parse_location_blob()` stub, simplifies `get_history()` to a `limit`-only API with clarified docs, and adjusts `lock()` sanitization to re-collapse whitespace after removing unsafe characters.
`fmd_api/client.py`	Documents behavior and exceptions for key public methods, adds `decrypt_data_blob_async()` using a thread executor, and clarifies semantics for location/picture retrieval and command-sending helpers.
`fmd_api/_version.py`	Updates the internal version constant to `2.0.8` to match `pyproject.toml`.
`fmd_api/__init__.py`	Exports `Location`, `PhotoResult`, and `RateLimitError` at the package root to improve discoverability and match Home Assistant expectations.
`docs/HOME_ASSISTANT_REVIEW.md`	Marks several review items as fixed (async decryption, changelog, exception documentation, exports, coverage) and updates the final checklist to reflect the current project status.
`CHANGELOG.md`	Introduces a Keep a Changelog–style history documenting releases from `2.0.0` through `2.0.8`, including the new async decryption, API exports, and behavior changes in this release.

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

Copilot · 2026-02-01T13:48:44Z

fmd_api/device.py

+    async def get_history(self, limit: int = -1) -> AsyncIterator[Location]:
        """
        Iterate historical locations. Uses client.get_locations() under the hood.
-        Yields decrypted Location objects newest-first (matches get_all_locations when requesting N recent).
+
+        Args:
+            limit: Maximum number of locations to return. -1 for all available.
+
+        Yields:
+            Decrypted Location objects, newest-first.
+
+        Raises:
+            OperationError: If decryption or parsing fails for a location blob.
+


Device.get_history() still calls the synchronous decrypt_data_blob() inside an async iterator, so decrypting many location blobs can block the event loop, even though FmdClient now exposes decrypt_data_blob_async() specifically to avoid this. If Device is intended for use in event-loop-sensitive contexts (like Home Assistant), consider adding an async decryption path here (e.g., via the async wrapper or asyncio.to_thread) or at least documenting the potential blocking behavior.

Copilot · 2026-02-01T13:48:44Z

docs/HOME_ASSISTANT_REVIEW.md

 **For Best Practices (Minor):**
 - Add CI badges — PARTIAL (Added Tests + Codecov badges; PyPI/version badges pending)
- Create CHANGELOG.md
- Document exceptions
- Add test coverage reporting
- Export all public models
+- Create CHANGELOG.md — DONE
+- Document exceptions — DONE
+- Add test coverage reporting — DONE (100% branch coverage)
+- Export all public models — DONE


This section marks "Add test coverage reporting" as DONE (100% branch coverage), but earlier in the same document (section 18: "No Test Coverage Reporting") the status is still listed as ❌ TODO. To avoid confusion when tracking Home Assistant review items, it would be good to update the earlier section's status or clarify the distinction (e.g., measurement vs. badges) so the checklist is internally consistent.

Copilot · 2026-02-01T13:48:45Z

fmd_api/client.py

+        Raises:
+            FmdApiException: If private key not loaded, blob too small, or decryption fails.
+        """
+        loop = asyncio.get_event_loop()


decrypt_data_blob_async currently uses asyncio.get_event_loop(), which is deprecated in modern Python when called from within a coroutine and may emit deprecation warnings. For async code paths it's safer and more future-proof to use asyncio.get_running_loop() instead before calling run_in_executor.

Suggested change

loop = asyncio.get_event_loop()

loop = asyncio.get_running_loop()

Copilot · 2026-02-01T13:48:45Z

fmd_api/client.py

+
+        Raises:
+            FmdApiException: If server returns an error or unexpected response.
+            aiohttp.ClientError: If a network error occurs.


The Raises section here documents aiohttp.ClientError, but get_locations() only performs HTTP calls through _make_api_request(), which wraps all aiohttp.ClientError instances into FmdApiException. As written, callers will never see a bare aiohttp.ClientError, so either the documentation should drop that entry (and mention only FmdApiException/asyncio.TimeoutError), or _make_api_request() should be adjusted to let aiohttp.ClientError propagate if you want that type to be part of the public API surface.

Suggested change

aiohttp.ClientError: If a network error occurs.

Copilot · 2026-02-01T13:48:45Z

fmd_api/client.py

+            List of picture blobs (may be base64 strings or metadata dicts).
+
+        Raises:
+            aiohttp.ClientError: If a network error occurs (returns empty list).


The get_pictures() docstring claims it raises aiohttp.ClientError "(returns empty list)", but the implementation catches aiohttp.ClientError and unconditionally returns an empty list, so no such exception is actually raised. To avoid confusing callers, consider either removing aiohttp.ClientError from the Raises section or changing the code to let it propagate instead of swallowing it.

Suggested change

List of picture blobs (may be base64 strings or metadata dicts).

Raises:

aiohttp.ClientError: If a network error occurs (returns empty list).

List of picture blobs (may be base64 strings or metadata dicts), or an empty

list if a network or HTTP error occurs.

Copilot · 2026-02-01T13:48:45Z

fmd_api/client.py

+            FmdApiException: If authentication fails or server returns an error.
+            aiohttp.ClientError: If a network error occurs.


The authenticate() docstring states that it can raise aiohttp.ClientError, but all HTTP calls go through _make_api_request(), which converts aiohttp.ClientError into FmdApiException. Unless there is another path that surfaces a raw aiohttp.ClientError, the Raises section should be updated to describe only FmdApiException and asyncio.TimeoutError for clarity.

Suggested change

FmdApiException: If authentication fails or server returns an error.

aiohttp.ClientError: If a network error occurs.

FmdApiException: If authentication fails or the server returns an error.

- Device.get_history() now uses decrypt_data_blob_async() to avoid blocking - Use asyncio.get_running_loop() instead of deprecated get_event_loop() - Fix Raises documentation: remove aiohttp.ClientError (wrapped by _make_api_request) - Fix get_pictures() docstring to clarify it returns empty list on error - Fix inconsistent status in HOME_ASSISTANT_REVIEW.md section 18 https://claude.ai/code/session_019KoQsx1g9tLyTsu2JakAnn

claude added 3 commits February 1, 2026 04:11

Copilot AI review requested due to automatic review settings February 1, 2026 13:44

Copilot started reviewing on behalf of devinslick February 1, 2026 13:44 View session

Copilot AI reviewed Feb 1, 2026

View reviewed changes

devinslick merged commit 01cae81 into main Feb 1, 2026
23 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Address Home Assistant Core review findings #17

Address Home Assistant Core review findings #17

Uh oh!

devinslick commented Feb 1, 2026 •

edited

Loading

Uh oh!

codecov bot commented Feb 1, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 1, 2026

Uh oh!

Copilot AI Feb 1, 2026

Uh oh!

Copilot AI Feb 1, 2026

Uh oh!

Copilot AI Feb 1, 2026

Uh oh!

Copilot AI Feb 1, 2026

Uh oh!

Copilot AI Feb 1, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	loop = asyncio.get_event_loop()
	loop = asyncio.get_running_loop()

		FmdApiException: If authentication fails or server returns an error.
		aiohttp.ClientError: If a network error occurs.

Address Home Assistant Core review findings #17

Address Home Assistant Core review findings #17

Uh oh!

Conversation

devinslick commented Feb 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Feb 1, 2026

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 1, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

devinslick commented Feb 1, 2026 •

edited

Loading