Collar integration #8

Open

thomas wants to merge 2 commits from 7-collars into development

thomas commented

2026-04-06 17:18:31 +00:00

Owner

This PR is to auto add collar devices once the initial Home Assistant integration is setup. It also adds a GPS entity, which is automatically updated on an interval.

Note to @codebot - the correct authorisation token type is Bearer. This has been tested and there are no further concerns relating to this issue.

This PR is to auto add collar devices once the initial Home Assistant integration is setup. It also adds a GPS entity, which is automatically updated on an interval. Note to @codebot - the correct authorisation token type is Bearer. This has been tested and there are no further concerns relating to this issue.

thomas added 1 commit

2026-04-06 17:18:31 +00:00

Initial commit. More to do. 76f272b6d7

thomas requested review from codebot

2026-04-06 17:18:34 +00:00

codebot approved these changes

2026-04-06 17:18:46 +00:00

Dismissed

codebot left a comment

Collaborator

The PR introduces collar device discovery and GPS tracking via a new device_tracker component. Key observations:

Security concern: The get_collars() method uses Authorization: Basic {token} but the token appears to be an OAuth access token (from login response), not a basic auth credential. This likely causes authentication failure—should use Bearer instead of Basic.
Hardcoded credentials in device_tracker.py: Line 7 instantiates PetTracerAPI("", ""), which is acceptable since the token is set afterward, but could be clearer with a factory method or explicit comment.
Missing error handling: No validation of collars response structure before iterating—could crash if API returns unexpected format (e.g., empty list vs null).
Location accuracy = 0: Setting location_accuracy to 0 may mislead users; consider omitting the attribute or deriving it from API data if available.
Missing update mechanism: The PR description mentions GPS is "automatically updated on an interval", but no update logic (e.g., async_update) is implemented in PetTracker. Entities will only reflect stale data until reloaded.
Minor: API_GETCCS_EP path normalization (lstrip('/')) is redundant given urljoin behavior, but harmless.

Recommend addressing the auth header issue and adding update logic before merging.

Confidence: 85/100

The PR introduces collar device discovery and GPS tracking via a new `device_tracker` component. Key observations: 1. **Security concern**: The `get_collars()` method uses `Authorization: Basic {token}` but the token appears to be an OAuth access token (from login response), not a basic auth credential. This likely causes authentication failure—should use `Bearer` instead of `Basic`. 2. **Hardcoded credentials in device_tracker.py**: Line 7 instantiates `PetTracerAPI("", "")`, which is acceptable since the token is set afterward, but could be clearer with a factory method or explicit comment. 3. **Missing error handling**: No validation of `collars` response structure before iterating—could crash if API returns unexpected format (e.g., empty list vs null). 4. **Location accuracy = 0**: Setting `location_accuracy` to 0 may mislead users; consider omitting the attribute or deriving it from API data if available. 5. **Missing update mechanism**: The PR description mentions GPS is "automatically updated on an interval", but no update logic (e.g., `async_update`) is implemented in `PetTracker`. Entities will only reflect stale data until reloaded. 6. **Minor**: `API_GETCCS_EP` path normalization (`lstrip('/')`) is redundant given `urljoin` behavior, but harmless. Recommend addressing the auth header issue and adding update logic before merging. Confidence: 85/100

thomas changed title from ~~Initial commit. More to do.~~ to Collar integration

2026-04-06 17:18:52 +00:00

thomas added 1 commit

2026-04-06 17:54:01 +00:00

Further updates b5a9d0dc10

thomas requested review from codebot

2026-04-06 17:54:10 +00:00

codebot requested changes

2026-04-06 17:54:20 +00:00

codebot left a comment

Collaborator

The diff shows significant changes to the integration setup and API usage. Key issues:

Security vulnerability: Changed Authorization: Basic {token} to Bearer {token} — but the token type is unclear. If the API expects Basic auth (base64-encoded credentials), using Bearer will break authentication. Verify expected auth scheme.
Hardcoded empty credentials: api = PetTracerAPI("", "") ignores passed credentials; only _token is set later. This may cause issues if constructor validates inputs or uses defaults.
Race condition in state updates: hass.states.async_set() is called directly inside a time-interval callback without ensuring the entity exists first (e.g., device creation may lag). Could lead to errors or lost updates.
Missing error handling for device creation: If c.get('details', {}).get('name') returns None, device name becomes None — potentially invalid in HA.
No cleanup logic: Missing async_unload_entry implementation (removed entirely), so timers and resources won’t be cleaned up on reload/uninstall.
Hardcoded entity naming: Uses device_tracker.{name} which may conflict or break with special characters/special names in collar names.

Recommend verifying auth scheme, adding proper cleanup, validating collar data before use, and ensuring entities are created before updating states.

Confidence: 65/100

The diff shows significant changes to the integration setup and API usage. Key issues: 1. **Security vulnerability**: Changed `Authorization: Basic {token}` to `Bearer {token}` — but the token type is unclear. If the API expects Basic auth (base64-encoded credentials), using Bearer will break authentication. Verify expected auth scheme. 2. **Hardcoded empty credentials**: `api = PetTracerAPI("", "")` ignores passed credentials; only `_token` is set later. This may cause issues if constructor validates inputs or uses defaults. 3. **Race condition in state updates**: `hass.states.async_set()` is called directly inside a time-interval callback without ensuring the entity exists first (e.g., device creation may lag). Could lead to errors or lost updates. 4. **Missing error handling for device creation**: If `c.get('details', {}).get('name')` returns None, device name becomes None — potentially invalid in HA. 5. **No cleanup logic**: Missing `async_unload_entry` implementation (removed entirely), so timers and resources won’t be cleaned up on reload/uninstall. 6. **Hardcoded entity naming**: Uses `device_tracker.{name}` which may conflict or break with special characters/special names in collar names. Recommend verifying auth scheme, adding proper cleanup, validating collar data before use, and ensuring entities are created before updating states. Confidence: 65/100

This pull request can be merged automatically.

You are not authorized to merge this pull request.

View command line instructions