Skip to content

Fix Aqara FP1E using incorrect manufacturer code#4749

Merged
TheJulianJES merged 1 commit intozigpy:devfrom
TheJulianJES:tjj/fix_aqara_fp1e_manufacturer_code
Feb 13, 2026
Merged

Fix Aqara FP1E using incorrect manufacturer code#4749
TheJulianJES merged 1 commit intozigpy:devfrom
TheJulianJES:tjj/fix_aqara_fp1e_manufacturer_code

Conversation

@TheJulianJES
Copy link
Collaborator

@TheJulianJES TheJulianJES commented Feb 12, 2026

Proposed change

This changes the attribute definition's manufacturer codes to None for the Aqara FP1E mm-wave sensor.
Otherwise, we aren't parsing the attribute reports from the device, since they don't have the manufacturer bit set.
For writes, it looks like the manufacturer code doesn't matter at all(??)

Additional information

Should address home-assistant/core#162484

Device diagnostics

zha-01KAH6BZ8GBKBJXCPX8V4BB4KP-Aqara Presence Sensor FP1E-73e318d90a5606426bd23584f7e84776.json

Checklist

  • The changes are tested and work correctly
  • pre-commit checks pass / the code has been formatted using Black
  • Tests have been added to verify that the new code works
  • Device diagnostics data has been attached

@codecov
Copy link

codecov bot commented Feb 12, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.32%. Comparing base (a49b834) to head (0d59286).
⚠️ Report is 2 commits behind head on dev.

Additional details and impacted files
@@           Coverage Diff           @@
##              dev    #4749   +/-   ##
=======================================
  Coverage   92.32%   92.32%           
=======================================
  Files         371      371           
  Lines       12195    12195           
=======================================
  Hits        11259    11259           
  Misses        936      936           

☔ 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.

@TheJulianJES
Copy link
Collaborator Author

Z2M ignores the manufacturer code for attribute reports. For writing, it's using it for this device for both the restart and spatial learning writes (manufacturerOptions.lumi): https://github.com/Koenkk/zigbee-herdsman-converters/blob/e410903af3516701e48d4a851cda7488f70f928d/src/lib/lumi.ts#L3198-L3225

However, using both buttons in ZHA seems to work fine without the manufacturer code/bit, so I think we're actually doing this more correct with this PR, though the sensor seems to ignore it anyway.

@TheJulianJES TheJulianJES marked this pull request as ready for review February 12, 2026 16:24
Copilot AI review requested due to automatic review settings February 12, 2026 16:24
@TheJulianJES
Copy link
Collaborator Author

Paired an FP1E on HA 2026.2.1, reproduced the issue, applied the fix from this PR, and could no longer reproduce it.
This should be good to go.

Copy link

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

This PR fixes the Aqara FP1E (lumi.sensor_occupy.agl1) presence sensor quirk to correctly handle attribute reports from the device. The device sends manufacturer-specific attribute reports without the manufacturer code bit set, which prevented ZHA from parsing them correctly. The fix migrates from the deprecated is_manufacturer_specific=True pattern to the modern manufacturer_code=None approach.

Changes:

  • Replaced is_manufacturer_specific=True with manufacturer_code=None for all 7 manufacturer-specific attributes in the OppleCluster class

@TheJulianJES
Copy link
Collaborator Author

TheJulianJES commented Feb 12, 2026

Ok, so I wasn't sure why this actually broke now, as you mentioned we still somewhat process attribute reports if the manufacturer code doesn't match the definition, but the reason is kind of stupid if I'm correct:

From what I understand, we took this path (without the PR):

  • zigpy/zcl/init.py#L861-L864
    find_attribute raises KeyError as report is mfr-specific but definition is not, so attr_def is None
  • https://github.com/zigpy/zigpy/blob/0ceeb47a324f911fda830e23312fd47a4162d136/zigpy/zcl/__init__.py#L1184
    _update_attribute searches attribute by ID only, so finds the correct definition (with mf-code) and updates the "typed cache"
  • zigpy/zcl/init.py#L866-L880
    AttributeReportedEvent is emitted from handle_cluster_general_request, with the unmodified value, but this doesn't matter as ZHA sensors only need the event and get the state from the attribute cache using cluster.get.
    That only works with the "new typed cache", but that's where the attribute value lives due to the more loose matching in _update_attribute.
    • However, for the ZHA sensor to actually check for updated state and possibly emit an event, it checks the attribute_name of the ClusterAttributeUpdatedEvent (just passed through from zigpy). The check is here: zha/application/platforms/sensor/init.py#L336
    • So, the sensor attribute_name needs to match the event from the zigpy event. And the zigpy event is always fired with attribute_name=None if it can't find the definition in handle_cluster_general_request.
      See: zigpy/zcl/init.py#L873

This isn't great because we first match with manufacturer code and fail to find a definition, but update_attribute matches without it and then finds the definition (that has a manufacturer code definition), even though that's somewhat incorrect.
Later, the attribute update event is emitted without the attribute_name and with the originally reported value, even though we store the value is the new typed runtime cache and with mf-code in the DB.

That should be somewhat improved with zigpy/zigpy#1760 and TheJulianJES/zigpy@tjj/manufacturer_update_fix...TheJulianJES:zigpy:tjj/manufacturer_update_fix_with_mfr_attr_fix_zcl_cache. But I think it still needs to be looked at afterwards. The behavior will be more consistent with those PRs, but yeah. Maybe not fully perfect yet if we can't find a definition.

@TheJulianJES TheJulianJES merged commit 2b3e53f into zigpy:dev Feb 13, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants