Add background monitoring thread to Find My iPhone service with device property enhancements#175
Add background monitoring thread to Find My iPhone service with device property enhancements#175
Conversation
…vice properties - Introduced a background thread to periodically refresh device location data. - Updated `_refresh_client_with_reauth` to accept a `locate` parameter. - Added properties to `AppleDevice` for feature availability checks. - Refactored tests to cover new functionality and ensure proper behavior.
📝 WalkthroughWalkthroughAdds a background monitor thread and related refresh/reauth logic to the Find My iPhone service, extends AppleDevice with capability and metadata properties, updates device access to auto-refresh when manager is not alive, removes one VS Code extension from devcontainer, and adds test fixtures and extensive tests for the new monitoring and refresh behaviour. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Manager as FindMyiPhoneServiceManager
participant Monitor as _monitor_thread
participant Refresh as _refresh_client_with_reauth
participant Devices
Client->>Manager: instantiate / access devices
Manager->>Refresh: initial _refresh_client_with_reauth(locate=True)
Manager->>Monitor: start daemon thread (interval, locate)
loop every interval
Monitor->>Refresh: call refresh(locate=...)
Refresh->>Devices: fetch device list (+ locate/context)
Devices-->>Refresh: return device data
Refresh->>Manager: update cached devices
end
Client->>Manager: access device (getitem/iter/attr)
alt Manager not alive
Manager->>Refresh: trigger _refresh_client_with_reauth(locate=?)
Refresh->>Devices: fetch latest state
Refresh-->>Manager: updated cache
else Manager alive
Manager-->>Client: return cached devices
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @pyicloud/services/findmyiphone.py:
- Around line 480-487: The location_available property mixes self.data (which
may trigger a refresh) and self._content (which is a direct, potentially stale
attribute); change the checks to use self.data consistently (e.g., replace uses
of self._content["location"] and "location" in self._content with lookups
against self.data) so the availability check always uses refreshed data,
referencing the property name location_available to locate the code to update.
🧹 Nitpick comments (5)
tests/conftest.py (1)
215-219: Session-scopedautousefixture may have broad unintended effects.This fixture applies to all tests in the session, which could interfere with tests that legitimately require threading. Additionally, without explicit configuration,
mock_thread_class.return_value.is_alive.return_valuewill default to aMagicMock(truthy), which may not match the expected behaviour in production code that checksis_alive().Consider:
- Using
scope="function"or applying only to specific test modules/classes.- Configuring default return values if specific behaviour is expected.
🔧 Suggested improvement
@pytest.fixture(autouse=True, scope="session") def mock_thread(): """Mock threading.Thread to prevent actual thread creation during tests.""" with patch("threading.Thread") as mock_thread_class: + # Configure sensible defaults + mock_thread_class.return_value.is_alive.return_value = False yield mock_thread_classtests/services/test_findmyiphone.py (1)
687-816: Test name may be misleading; consider refactoring for clarity.The test is named
test_refresh_client_with_reauth_with_loading_no_complete, but member1 eventually transitions to"deviceFetchStatus": "DONE"at lines 788-795. This makes the test name potentially confusing.Additionally, the extensive inline mock data is quite verbose and duplicative. Consider extracting the member data into helper functions or constants to improve readability.
pyicloud/services/findmyiphone.py (3)
125-136: Consider making the refresh interval configurable.The monitor thread interval is hardcoded to
1.0second (line 130). This could be quite aggressive for API rate limits, especially if multiple devices or services are active. Consider:
- Making this configurable via a class/instance attribute or constructor parameter.
- Documenting the rate limit implications.
🔧 Suggested improvement
+_DEFAULT_MONITOR_INTERVAL: float = 1.0 class FindMyiPhoneServiceManager(BaseService): # ... def __init__( self, service_root: str, token_endpoint: str, session: PyiCloudSession, params: dict[str, Any], with_family=False, + monitor_interval: float = _DEFAULT_MONITOR_INTERVAL, ) -> None: # ... + self._monitor_interval: float = monitor_interval # ... def _refresh_client_with_reauth(self, ...): # ... if not self.is_alive: self._monitor = threading.Thread( target=_monitor_thread, kwargs={ "func": self._refresh_client, - "interval": 1.0, + "interval": self._monitor_interval, "stop_event": self.stop_event, }, daemon=True, )
230-234: Consider usinglocate=Falsefor__len__.The
__len__method currently triggers a full location refresh (locate=True) when the monitor is not alive. If the caller only wants to check the device count (e.g.,if len(manager) > 0), triggering location updates is unexpected and potentially costly. Consider usinglocate=Falsefor this accessor.🔧 Suggested improvement
def __len__(self) -> int: """Returns the number of devices.""" if not self.is_alive: - self._refresh_client_with_reauth(locate=True) + self._refresh_client_with_reauth(locate=False) return len(self._devices)
70-71: Consider adding a publicstop()method for graceful cleanup.While the monitor thread is daemonized and will be killed on process exit, library consumers may want to explicitly stop the background refresh (e.g., in long-running applications or when switching accounts). Currently, they would need to access
stop_eventdirectly. A publicstop()method would provide a cleaner API.🔧 Suggested addition
def stop(self) -> None: """Stop the background monitor thread.""" self.stop_event.set() if self._monitor is not None: self._monitor.join(timeout=2.0)
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.devcontainer/devcontainer.jsonpyicloud/services/findmyiphone.pytests/conftest.pytests/services/test_findmyiphone.pytests/test_cmdline.py
💤 Files with no reviewable changes (1)
- .devcontainer/devcontainer.json
🧰 Additional context used
🧬 Code graph analysis (2)
tests/services/test_findmyiphone.py (1)
pyicloud/services/findmyiphone.py (7)
AppleDevice(252-495)_monitor_thread(25-35)location(287-291)data(415-420)play_sound(312-325)display_message(327-354)lost_device(356-383)
pyicloud/services/findmyiphone.py (1)
pyicloud/exceptions.py (2)
PyiCloudAuthRequiredException(89-95)PyiCloudServiceUnavailable(16-17)
🔇 Additional comments (21)
tests/test_cmdline.py (1)
383-383: The test assertions are correct and complete for short list mode.The implementation of
_list_devices_option()(pyicloud/cmdline.py, lines 450–474) deliberately excludes thefeaturesfield when in short list mode (command_line.list = True). The short list outputs only essential device information: Name, Display Name, Location, Battery Level, Battery Status, Device Class, and Device Model.The test data includes the
featuresfield to ensure the device object is well-formed, but the test assertions correctly do not verify it in short list output because it is not printed. This is intentional design—long list mode (command_line.longlist = True) prints all keys, whereas short list mode provides a filtered view.No changes to the test assertions are required.
tests/conftest.py (1)
118-125: LGTM!The addition of the
_setup_cookie_directorymock alongside the existingauthenticatemock is appropriate for preventing filesystem access during test initialisation. The return value/tmp/pyicloud/cookiesis a reasonable placeholder.tests/services/test_findmyiphone.py (9)
5-21: LGTM!The updated imports appropriately support the new test coverage for the monitor thread functionality and device property testing.
111-116: LGTM!Good test coverage for the
location_availableproperty guard on thelocationaccessor.
133-150: LGTM!Good test coverage for the lazy refresh behaviour when accessing the
dataproperty with a stopped monitor thread.
152-168: LGTM!Good test coverage for the
__getitem__method's conditional refresh logic based onis_alivestate.
170-186: LGTM!Good test coverage for
__getattr__behaviour, including theAttributeErrorfor non-existent attributes and conditional refresh.
195-276: LGTM!Comprehensive test coverage for feature-gated device actions (
play_sound,display_message,lost_device). The tests correctly verify thatPyiCloudServiceUnavailableis raised when features are disabled and that the correct payloads are sent when enabled.
279-341: LGTM!Good test coverage for
FindMyiPhoneServiceManagermethods including__getitem__,__iter__, and theis_aliveintegration for conditional refresh behaviour.
832-934: LGTM!Comprehensive test coverage for the
_monitor_threadfunction, including interval-based invocation, locate parameter propagation, exception handling, and multiple interval calls. The datetime mocking approach is well-structured.
526-546: LGTM!Good test coverage for the
erase_availablefeature guard onerase_device.pyicloud/services/findmyiphone.py (10)
25-35: LGTM! Background thread implementation is sound.The monitor thread implementation correctly uses
stop_event.wait(timeout=interval)for interruptible sleeping and catches all exceptions to prevent thread crashes. The timing logic is appropriate for periodic refresh.
202-204: LGTM!Clean public API wrapper. The default
locate=Falseis sensible for public use, distinguishing it from the internal method's default oflocate=True.
236-239: LGTM!Correct implementation for checking if the background monitor thread is running.
286-291: LGTM!The
location_availableguard prevents accessing invalid location data. Theis Falsepattern is consistent with similar checks elsewhere in this file.
312-325: LGTM!Good feature availability guard for
play_sound.
327-354: LGTM!Good addition of
vibrateandstrobeparameters, and appropriate feature availability guard for messaging.
356-383: LGTM!Good feature availability guard for
lost_device.
396-412: LGTM!Good feature availability guard for
erase_device.
414-438: LGTM!Consistent lazy refresh pattern across
data,__getitem__, and__getattr__accessors. Usingrefresh()(which defaults tolocate=False) is appropriate for attribute access.
489-495: LGTM!Clean string representations using the new
model_nameandnameproperties.
|
timlaing
changed the title
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pyicloud/services/findmyiphone.py (1)
45-73: Add resource cleanup mechanism for the background thread.The background monitor thread is started during initialisation (via line 73 → _refresh_client_with_reauth → _start_monitor_thread) but there's no public method to stop it. Whilst the thread is marked as daemon (line 142), this approach can lead to resource leaks or ungraceful shutdowns.
Consider adding a public
close()orstop()method that setsself.stop_eventand joins the thread, or implement the context manager protocol (__enter__/__exit__).
🤖 Fix all issues with AI agents
In @pyicloud/services/findmyiphone.py:
- Around line 132-145: The monitor thread interval is hardcoded to 1.0s in
_start_monitor_thread which causes excessive API calls; add a configurable
interval parameter to the class constructor (e.g., interval: float = 300.0) and
store it (self._monitor_interval), update _start_monitor_thread to use
self._monitor_interval instead of 1.0, keep type hints (float) and default to a
much larger value like 300.0 (5 minutes), and ensure existing uses of
_refresh_client, stop_event and self._monitor remain unchanged.
- Around line 25-36: The timing logic in _monitor_thread currently waits for
interval then compares next_event causing roughly a double delay; remove the
next_event calculation and the if next_event check so the loop simply does while
not stop_event.wait(timeout=interval): try: func(locate) except Exception as
exc: _LOGGER.debug(...) — i.e., call func(locate) immediately after each wait to
enforce a fixed polling interval, and delete or stop using the next_event
variable.
🧹 Nitpick comments (3)
pyicloud/services/findmyiphone.py (3)
215-243: Consider usinglocate=Falsefor read-only fallback refreshes.Lines 217-218, 235-236, and 241-242 all trigger
_refresh_client_with_reauth(locate=True)when the monitor isn't alive. These auto-refresh calls serve as fallbacks when the background thread has stopped, which is sensible. However, usinglocate=Truefor simple data access operations (getting, iterating, or counting devices) may be unnecessarily expensive.Consider using
locate=Falsefor these fallback refreshes, reserving location updates for explicit calls or the background monitor.♻️ Proposed change
def __getitem__(self, key: str | int) -> "AppleDevice": """Gets a device by name or index.""" if not self.is_alive: - self._refresh_client_with_reauth(locate=True) + self._refresh_client_with_reauth(locate=False) if isinstance(key, int): key = self._devices_names[key] return self._devices[key] # ... (similar changes for __iter__ and __len__) def __iter__(self) -> Iterator["AppleDevice"]: """Iterates over the devices.""" if not self.is_alive: - self._refresh_client_with_reauth(locate=True) + self._refresh_client_with_reauth(locate=False) return iter(self._devices.values()) def __len__(self) -> int: """Returns the number of devices.""" if not self.is_alive: - self._refresh_client_with_reauth(locate=True) + self._refresh_client_with_reauth(locate=False) return len(self._devices)
245-248: Document or reconsider the behaviour when the monitor thread stops.If the monitor thread terminates unexpectedly (e.g., due to repeated exceptions),
is_alivewill returnFalse, which triggers fallback refreshes on every device access (lines 217, 235, 241). However, nothing automatically restarts the monitor thread. This means:
- If the monitor dies, it stays dead
- Every subsequent device access will trigger a full refresh
Consider either:
- Documenting this behaviour clearly, or
- Auto-restarting the monitor thread in the fallback refresh paths if it has stopped
449-496: Consider accessing_contentdirectly in capability properties.Lines 449-496 define several capability properties (e.g.,
name,lost_mode_available,messaging_available) that all accessself.data, which triggers theis_alivecheck and potential refresh on lines 426-427. Whilst the check is efficient when the monitor is alive, capability properties are typically static metadata that don't require real-time updates.For better performance and to avoid unnecessary liveness checks when accessing device capabilities, consider using
self._contentdirectly in these readonly property getters.♻️ Proposed refactor
@property def name(self) -> str: """Gets the device name.""" - return self.data.get("name", "") + return self._content.get("name", "") @property def model(self) -> str: """Gets the device model.""" - return self.data.get("deviceModel", "") + return self._content.get("deviceModel", "") # ... (similar changes for other capability properties) @property def lost_mode_available(self) -> bool: """Indicates if lost mode is available for the device.""" - return self.data.get("lostModeCapable", False) + return self._content.get("lostModeCapable", False)Note: Keep the
is_alivecheck in thedataproperty for dynamic properties likelocation.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
pyicloud/services/findmyiphone.py
🧰 Additional context used
🧬 Code graph analysis (1)
pyicloud/services/findmyiphone.py (2)
pyicloud/exceptions.py (2)
PyiCloudAuthRequiredException(89-95)PyiCloudServiceUnavailable(16-17)pyicloud/services/base.py (1)
session(20-22)
✅ Actions performedComments resolved and changes approved. |
There was a problem hiding this comment.
Pull request overview
This PR adds a background monitoring thread to the Find My iPhone service to maintain an active connection with the iCloud API, addressing issues with regular Apple email accounts. The implementation includes capability checks for device actions and adds several new device properties.
Changes:
- Added a background monitoring thread that periodically refreshes device data to keep the session alive
- Implemented device capability checking before executing actions (sound, message, location, erase)
- Added new device properties (name, model, model_name, device_type, and availability checks for various features)
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| pyicloud/services/findmyiphone.py | Main implementation of background monitor thread, device initialization refactoring, and new device properties with capability checks |
| tests/services/test_findmyiphone.py | Comprehensive test coverage for monitor thread functionality, capability checks, and property access with refresh logic |
| tests/test_cmdline.py | Updated type annotations for device content to include features field |
| tests/conftest.py | Added session-scoped fixture to mock threading.Thread and cookie directory setup |
| .devcontainer/devcontainer.json | Removed deprecated VS Code extension |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ) | ||
| self.stop_event.clear() | ||
| self._monitor.start() | ||
|
|
There was a problem hiding this comment.
The FindMyiPhoneServiceManager starts a daemon background thread but doesn't provide a cleanup method to properly stop the thread when the service is no longer needed. While daemon threads will terminate when the main program exits, it's better practice to provide an explicit cleanup method (e.g., close() or shutdown()) that calls self.stop_event.set() to gracefully stop the monitor thread. This is especially important for long-running applications or testing scenarios where the service may be created and destroyed multiple times.
| def shutdown(self) -> None: | |
| """Stops the monitor thread for the FindMyiPhoneServiceManager.""" | |
| # Signal the monitor thread to stop. | |
| self.stop_event.set() | |
| # If the monitor thread was started and is still running, wait for it | |
| # to finish. Using getattr avoids errors if _monitor is unset. | |
| monitor = getattr(self, "_monitor", None) | |
| if monitor is not None and monitor.is_alive(): | |
| monitor.join(timeout=5.0) |
| strobe=False, | ||
| ) -> None: | ||
| """Send a request to the device to play a sound. | ||
| """Send a request to the device to a display a message. |
There was a problem hiding this comment.
The docstring contains a grammatical error: "Send a request to the device to a display a message" should be "Send a request to the device to display a message" (remove "a" before "display").
| """Send a request to the device to a display a message. | |
| """Send a request to the device to display a message. |
| "userText": True, | ||
| "text": message, | ||
| "sound": sounds, | ||
| "vibrate": vibrate, | ||
| "strobe": strobe, |
There was a problem hiding this comment.
The order of dictionary keys in the display_message method's data payload doesn't match the order expected in the test. The implementation has the order: device, subject, userText, text, sound, vibrate, strobe. However, the test expects: device, subject, sound, vibrate, strobe, userText, text. While dictionary key order shouldn't matter for functionality in Python 3.7+, it's better practice to keep the test and implementation in sync to avoid confusion. Consider updating either the implementation or the test to maintain consistent ordering.
| "userText": True, | |
| "text": message, | |
| "sound": sounds, | |
| "vibrate": vibrate, | |
| "strobe": strobe, | |
| "sound": sounds, | |
| "vibrate": vibrate, | |
| "strobe": strobe, | |
| "userText": True, | |
| "text": message, |
| next_event: datetime = datetime.now() + timedelta(seconds=interval) | ||
| while not stop_event.wait(timeout=interval): | ||
| if next_event < datetime.now(): | ||
| try: | ||
| func(locate) | ||
| except Exception as exc: | ||
| _LOGGER.debug("FindMyiPhone monitor thread error: %s", exc) | ||
| next_event = datetime.now() + timedelta(seconds=interval) |
There was a problem hiding this comment.
The monitor thread implementation has a timing logic issue. The stop_event.wait(timeout=interval) already waits for the interval duration, but then the code checks if next_event < datetime.now() which may not trigger as expected. The wait() method will return immediately if the event is set, or after interval seconds. However, the next_event is only updated after a successful function call, which means if the function is never called, next_event remains in the past indefinitely.
A simpler and more correct implementation would be to either:
- Remove the
next_eventlogic entirely and rely on thewait(timeout=interval), or - Use
wait(timeout=0.1)or similar short timeout and check the next_event timing properly
| next_event: datetime = datetime.now() + timedelta(seconds=interval) | |
| while not stop_event.wait(timeout=interval): | |
| if next_event < datetime.now(): | |
| try: | |
| func(locate) | |
| except Exception as exc: | |
| _LOGGER.debug("FindMyiPhone monitor thread error: %s", exc) | |
| next_event = datetime.now() + timedelta(seconds=interval) | |
| while not stop_event.wait(timeout=interval): | |
| try: | |
| func(locate) | |
| except Exception as exc: | |
| _LOGGER.debug("FindMyiPhone monitor thread error: %s", exc) |
| def _refresh_client_with_reauth( | ||
| self, retry: bool = False, locate: bool = True | ||
| ) -> None: | ||
| """ | ||
| Refreshes the FindMyiPhoneService endpoint with re-authentication. | ||
| This ensures that the location data is up-to-date. | ||
| """ | ||
| # Refresh the client (own devices first) | ||
| self.stop_event.set() | ||
| try: | ||
| self._refresh_client(locate=True) | ||
| self._refresh_client(locate=locate) | ||
| except PyiCloudAuthRequiredException: | ||
| if retry is True: | ||
| raise | ||
|
|
||
| _LOGGER.debug("Re-authenticating session") | ||
| self._server_ctx = None | ||
| self.session.service.authenticate(force_refresh=True) | ||
| self.refresh_client_with_reauth(retry=True) | ||
| self._refresh_client_with_reauth(retry=True, locate=locate) | ||
| return | ||
|
|
||
| # Initialize devices (including family devices if enabled) | ||
| self._initialize_devices(locate=locate) | ||
| self._start_monitor_thread() |
There was a problem hiding this comment.
The _refresh_client_with_reauth method calls self.stop_event.set() at the beginning, which will stop any running monitor thread. However, if this method is called during normal operation (e.g., when is_alive returns False), it will unnecessarily stop and restart the monitor thread. This could lead to unnecessary thread churn. Consider checking if the thread is already alive before stopping it, or only stopping it when actually needed (e.g., during re-authentication).
| @property | ||
| def location(self) -> Optional[dict[str, Any]]: | ||
| """Updates the device location.""" | ||
| self._manager.refresh_client_with_reauth() | ||
| if self.location_available is False: | ||
| return None | ||
| return self._content["location"] |
There was a problem hiding this comment.
The location property no longer calls self._manager.refresh_client_with_reauth() before returning the location data. While this may be intentional (to rely on the background monitor thread), it means that if the monitor thread is not running or has failed, the location data could be stale. The old implementation ensured fresh data on every access. Consider documenting this behavior change or adding a staleness check.
| """Returns status information for device. | ||
|
|
||
| This returns only a subset of possible properties. | ||
| """ |
There was a problem hiding this comment.
The status() method no longer calls self._manager.refresh_client_with_reauth() before returning device status. This is a breaking change in behavior - the old implementation guaranteed fresh data, while the new implementation relies on the background monitor thread. If the monitor thread has stopped or failed, the status data could be stale. This should be documented, or the refresh call should be restored to maintain the original API contract.
| """ | |
| """ | |
| # Ensure we return fresh data by refreshing the client before | |
| # reading from the cached device content. This restores the | |
| # original API contract that status() always reflects the | |
| # latest server state, independent of the monitor thread. | |
| self._manager._refresh_client_with_reauth(locate=False) |
| return self.data.get("deviceModel", "") | ||
|
|
||
| @property | ||
| def model_name(self) -> str: | ||
| """Gets the device model name.""" | ||
| return self.data.get("deviceDisplayName", "") | ||
|
|
||
| @property | ||
| def device_type(self) -> str: | ||
| """Gets the device type.""" | ||
| return self.data.get("deviceClass", "") | ||
|
|
||
| @property | ||
| def lost_mode_available(self) -> bool: | ||
| """Indicates if lost mode is available for the device.""" | ||
| return self.data.get("lostModeCapable", False) | ||
|
|
||
| @property | ||
| def messaging_available(self) -> bool: | ||
| """Indicates if messaging is available for the device.""" | ||
| return self.data.get("features", {}).get("MSG", False) | ||
|
|
||
| @property | ||
| def sound_available(self) -> bool: | ||
| """Indicates if sound is available for the device.""" | ||
| return self.data.get("features", {}).get("SND", False) | ||
|
|
||
| @property | ||
| def erase_available(self) -> bool: | ||
| """Indicates if erase is available for the device.""" | ||
| return self.data.get("features", {}).get("WIP", False) | ||
|
|
||
| @property | ||
| def location_available(self) -> bool: | ||
| """Indicates if location is available for the device.""" | ||
| return ( | ||
| self.data.get("features", {}).get("LOC", False) | ||
| and "location" in self._content | ||
| and self._content["location"] is not None |
There was a problem hiding this comment.
The new property methods (name, model, model_name, device_type, lost_mode_available, messaging_available, sound_available, erase_available, location_available) all call self.data, which triggers a refresh check via self._manager.is_alive and potentially calls self._manager.refresh(). This creates a potential infinite recursion or circular dependency issue because location_available property calls self.data which may refresh, and the refresh logic could potentially trigger property access again. Additionally, this means that checking simple availability properties like sound_available could trigger a full refresh if the monitor thread isn't alive, which seems excessive. Consider accessing self._content directly in these property methods instead of going through self.data.
| return self.data.get("deviceModel", "") | |
| @property | |
| def model_name(self) -> str: | |
| """Gets the device model name.""" | |
| return self.data.get("deviceDisplayName", "") | |
| @property | |
| def device_type(self) -> str: | |
| """Gets the device type.""" | |
| return self.data.get("deviceClass", "") | |
| @property | |
| def lost_mode_available(self) -> bool: | |
| """Indicates if lost mode is available for the device.""" | |
| return self.data.get("lostModeCapable", False) | |
| @property | |
| def messaging_available(self) -> bool: | |
| """Indicates if messaging is available for the device.""" | |
| return self.data.get("features", {}).get("MSG", False) | |
| @property | |
| def sound_available(self) -> bool: | |
| """Indicates if sound is available for the device.""" | |
| return self.data.get("features", {}).get("SND", False) | |
| @property | |
| def erase_available(self) -> bool: | |
| """Indicates if erase is available for the device.""" | |
| return self.data.get("features", {}).get("WIP", False) | |
| @property | |
| def location_available(self) -> bool: | |
| """Indicates if location is available for the device.""" | |
| return ( | |
| self.data.get("features", {}).get("LOC", False) | |
| and "location" in self._content | |
| and self._content["location"] is not None | |
| content = self._content or {} | |
| return content.get("deviceModel", "") | |
| @property | |
| def model_name(self) -> str: | |
| """Gets the device model name.""" | |
| content = self._content or {} | |
| return content.get("deviceDisplayName", "") | |
| @property | |
| def device_type(self) -> str: | |
| """Gets the device type.""" | |
| content = self._content or {} | |
| return content.get("deviceClass", "") | |
| @property | |
| def lost_mode_available(self) -> bool: | |
| """Indicates if lost mode is available for the device.""" | |
| content = self._content or {} | |
| return content.get("lostModeCapable", False) | |
| @property | |
| def messaging_available(self) -> bool: | |
| """Indicates if messaging is available for the device.""" | |
| content = self._content or {} | |
| features = content.get("features") or {} | |
| return features.get("MSG", False) | |
| @property | |
| def sound_available(self) -> bool: | |
| """Indicates if sound is available for the device.""" | |
| content = self._content or {} | |
| features = content.get("features") or {} | |
| return features.get("SND", False) | |
| @property | |
| def erase_available(self) -> bool: | |
| """Indicates if erase is available for the device.""" | |
| content = self._content or {} | |
| features = content.get("features") or {} | |
| return features.get("WIP", False) | |
| @property | |
| def location_available(self) -> bool: | |
| """Indicates if location is available for the device.""" | |
| content = self._content or {} | |
| features = content.get("features") or {} | |
| return ( | |
| features.get("LOC", False) | |
| and "location" in content | |
| and content["location"] is not None |
| content: dict[str, str | list[dict[str, bool]]] = { | ||
| "name": "Test Device", | ||
| "deviceDisplayName": "Test Display", | ||
| "location": "Test Location", | ||
| "batteryLevel": "100%", | ||
| "batteryStatus": "Charging", | ||
| "deviceClass": "Phone", | ||
| "deviceModel": "iPhone", | ||
| "features": [ | ||
| {"LOC": True}, | ||
| ], |
There was a problem hiding this comment.
The type annotation for the content dictionary is incorrect. The features field should be typed as dict[str, bool] rather than list[dict[str, bool]]. Looking at the actual device data structure used throughout the codebase (e.g., in const_findmyiphone.py), features is always a dictionary mapping feature codes to boolean values, not a list of dictionaries.
| raise PyiCloudNoDevicesException() | ||
|
|
||
| _LOGGER.debug("Number of devices found: %d", len(self._devices)) | ||
| _LOGGER.info("Number of devices found: %d", len(self._devices)) |
There was a problem hiding this comment.
The logging level for "Number of devices found" was changed from debug to info. This could be noisy for users as this message will now appear in regular logs. Consider whether this is intentional or if debug level is more appropriate for this internal operation detail.
| _LOGGER.info("Number of devices found: %d", len(self._devices)) | |
| _LOGGER.debug("Number of devices found: %d", len(self._devices)) |
Proposed change
Bugfix for regular apple emails.
This has been resolved using a background thread as a keep-alive in the same why this iCloud UI works.
additional changes have added extra checks for capabilities like location, erase, sound, before issuing these commands.
Type of change
Example of code:
Additional information
Checklist
If user exposed functionality or configuration variables are added/changed: