Skip to content

fix(akuvox): handle X916 user filtering where source_type is None#584

Merged
tykeal merged 2 commits intoFutureTense:mainfrom
tykeal:akuvox
Mar 12, 2026
Merged

fix(akuvox): handle X916 user filtering where source_type is None#584
tykeal merged 2 commits intoFutureTense:mainfrom
tykeal:akuvox

Conversation

@tykeal
Copy link
Copy Markdown
Collaborator

@tykeal tykeal commented Mar 12, 2026

Problem

The Akuvox provider filters local vs cloud users by checking source_type == "1". This works for A08S and E18C models, but the X916 firmware does not populate source_type at all (returns None for all users). This causes the provider to skip every user on X916 devices, breaking user code management entirely.

Observed symptoms on X916:

  • async_get_usercodes() returns empty list
  • async_set_usercode() always calls add_user instead of modify_user, causing FAILED errors when a user already exists
  • async_clear_usercode() silently does nothing

Solution

Replace the inline source_type check with a _is_local_user() helper that handles both firmware patterns:

Model Local user Cloud user
A08S / E18C source_type="1" source_type="2"
X916 source_type=None, user_type="-1" source_type=None, user_type="0"

Logic:

  1. If source_type is present (not None): check == "1"
  2. If source_type is None/absent: fall back to user_type == "-1"

Testing

  • 6 new unit tests for _is_local_user() covering all branches
  • 4 new X916-pattern integration tests across all CRUD methods
  • 75 total tests pass, 100% coverage on akuvox.py

Verified against real hardware

Tested against three device models:

  • A08S (FW 108.30.10.144) — source_type="1" for local users ✅
  • E18C (FW 18.30.10.111) — source_type="1"/"2" for local/cloud ✅
  • X916 (FW 916.30.10.114) — source_type=None, user_type="-1"/"0"

The X916 firmware does not populate source_type for any users. Instead
it uses user_type to distinguish local ("-1") from cloud ("0") users.
The A08S and E18C models use source_type "1" (local) / "2" (cloud).

Replace the inline source_type check with a _is_local_user() helper
that handles both firmware patterns:
  - source_type present: check == "1"
  - source_type absent/None: fall back to user_type == "-1"

Co-authored-by: Copilot <copilot@github.com>
Signed-off-by: Andrew Grimberg <tykeal@bardicgrove.org>
@tykeal tykeal requested a review from Copilot March 12, 2026 17:43
@github-actions github-actions Bot added the bugfix Fixes a bug label Mar 12, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes Akuvox user filtering so X916 devices (which return source_type=None) no longer have all users incorrectly excluded, restoring proper usercode CRUD behavior in the provider.

Changes:

  • Add _is_local_user() helper to classify local vs cloud users across multiple Akuvox firmware patterns.
  • Replace inline source_type checks in all CRUD paths with _is_local_user().
  • Extend the test suite with X916-pattern cases for filtering and CRUD flows.

Reviewed changes

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

File Description
custom_components/keymaster/providers/akuvox.py Introduces _is_local_user() and uses it throughout user filtering logic.
tests/providers/test_akuvox.py Adds unit/integration tests for X916/local-user classification and CRUD behavior.

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

Comment thread tests/providers/test_akuvox.py
Comment thread tests/providers/test_akuvox.py
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Mar 12, 2026

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.44%. Comparing base (cdb4922) to head (03c3973).
⚠️ Report is 61 commits behind head on main.
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #584      +/-   ##
==========================================
+ Coverage   84.14%   88.44%   +4.29%     
==========================================
  Files          10       26      +16     
  Lines         801     3141    +2340     
==========================================
+ Hits          674     2778    +2104     
- Misses        127      363     +236     
Flag Coverage Δ
python 88.44% <100.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

The edit that added test_get_code_skips_x916_cloud_users accidentally
swallowed the function definition of test_get_code_empty_pin, merging
its body into the X916 test. Restore it as a separate test function.

Co-authored-by: Copilot <copilot@github.com>
Signed-off-by: Andrew Grimberg <tykeal@bardicgrove.org>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.


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

@tykeal tykeal merged commit a6b8299 into FutureTense:main Mar 12, 2026
13 checks passed
@tykeal tykeal deleted the akuvox branch March 12, 2026 19:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix Fixes a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants