-
Notifications
You must be signed in to change notification settings - Fork 100
Fix and forward gpepIncomingMessageHandler for Green Power support #713
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Changes from 4 commits
fb7ffda
7eca82a
91c7c01
4b22bf3
f2578e8
4bc2d50
2df0ea5
af28f20
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,33 @@ | ||
| import bellows.types as t | ||
|
|
||
| from ..v14.commands import COMMANDS as COMMANDS_v14 | ||
|
|
||
| COMMANDS = { | ||
| **COMMANDS_v14, | ||
| # EZSP v16 appends an ``EmberRxPacketInfo`` struct after the LVBytes | ||
| # payload in ``gpepIncomingMessageHandler``. zigbee-herdsman gates the | ||
| # read on ``version >= 0x10`` (see | ||
| # ``src/adapter/ember/ezsp/ezsp.ts`` in the ``ember`` adapter), so v13 | ||
| # and v14 do not carry this trailer but v16 does. v17 and v18 override | ||
| # the whole command again, so this entry only matters for v16. | ||
|
puddly marked this conversation as resolved.
Outdated
|
||
| "gpepIncomingMessageHandler": ( | ||
| 0x00C5, | ||
| {}, | ||
| { | ||
| "status": t.uint8_t, | ||
| "gpdLink": t.uint8_t, | ||
| "sequenceNumber": t.uint8_t, | ||
| "addr": t.EmberGpAddress, | ||
| "gpdfSecurityLevel": t.EmberGpSecurityLevel, | ||
| "gpdfSecurityKeyType": t.EmberGpKeyType, | ||
| "autoCommissioning": t.Bool, | ||
| "bidirectionalInfo": t.uint8_t, | ||
| "gpdSecurityFrameCounter": t.uint32_t, | ||
| "gpdCommandId": t.uint8_t, | ||
| "mic": t.uint32_t, | ||
| "proxyTableIndex": t.uint8_t, | ||
| "gpdCommandPayload": t.LVBytes, | ||
| "packetInfo": t.SlRxPacketInfo, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've tested this with an adapter running EZSPv13 and EZSPv18 and parsing fails because |
||
| }, | ||
| ), | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -357,16 +357,43 @@ class EmberTokTypeStackZllSecurity(EzspStruct): | |
|
|
||
|
|
||
| class EmberGpAddress(EzspStruct): | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The SDK source shows the definition as: /**
* @brief GPD Address for sending and receiving a GPDF.
*/
typedef struct {
union {
/** The IEEE address is used when the application identifier is
* ::SL_ZIGBEE_GP_APPLICATION_IEEE_ADDRESS.
*/
sl_802154_long_addr_t gpdIeeeAddress;
/** The 32-bit source identifier is used when the application identifier is
* ::SL_ZIGBEE_GP_APPLICATION_SOURCE_ID.
*/
sl_zigbee_gp_source_id_t sourceId;
} id;
/** Application identifier of the GPD. */
sl_zigbee_gp_application_id_t applicationId;
/** Application endpoint , only used when application identifier is
* ::SL_ZIGBEE_GP_APPLICATION_IEEE_ADDRESS
*/
uint8_t endpoint;
} sl_zigbee_gp_address_t;This differs from the field order in this struct.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I tried matching the SDK order but the captured frame from @dmatscheko test fails to parse: the EZSP wire layout serializes applicationId first, even though the C struct declares id first. |
||
| # A GP address structure. | ||
| # The GPD's EUI64. | ||
| gpdIeeeAddress: named.EUI64 | ||
| # The GPD's source ID. | ||
| sourceId: basic.uint32_t | ||
| # The GPD Application ID. | ||
| """A GP address structure. | ||
|
|
||
| On the wire this is a 10-byte layout: | ||
|
|
||
| - ``applicationId`` (1 byte) — 0 means ``SrcID`` mode, 2 means ``IEEE`` | ||
| mode. | ||
| - ``id`` (8 bytes) — a raw union. When ``applicationId == 0`` the source | ||
| ID lives in the first 4 bytes (little-endian) and the remaining 4 | ||
| bytes are padding. When ``applicationId == 2`` the full 8 bytes are | ||
| the GPD EUI64. | ||
| - ``endpoint`` (1 byte). | ||
|
|
||
| The historical declaration here treated the union as two separate | ||
| fields (``gpdIeeeAddress`` + ``sourceId``) for a total of 14 bytes, | ||
| which does not match what the NCP sends in | ||
| ``gpepIncomingMessageHandler``. See the ``source_id`` and | ||
| ``gpd_ieee_address`` helpers below to access the right view. | ||
| """ | ||
|
|
||
| # The GPD Application ID: 0 = source ID mode, 2 = IEEE mode. | ||
| applicationId: basic.uint8_t | ||
| # Raw 8-byte union. Use :attr:`source_id` or :attr:`gpd_ieee_address` | ||
| # to interpret it based on :attr:`applicationId`. | ||
| id: basic.FixedList[basic.uint8_t, 8] | ||
| # The GPD endpoint. | ||
| endpoint: basic.uint8_t | ||
|
puddly marked this conversation as resolved.
Outdated
|
||
|
|
||
| @property | ||
| def source_id(self) -> int: | ||
| """Return the 32-bit source ID (only valid when applicationId == 0).""" | ||
| return int.from_bytes(bytes(self.id[:4]), "little") | ||
|
|
||
| @property | ||
| def gpd_ieee_address(self) -> named.EUI64: | ||
| """Return the EUI64 (only valid when applicationId == 2).""" | ||
| return named.EUI64(bytes(self.id)) | ||
|
|
||
|
|
||
| class NV3StackTrustCenterToken(EzspStruct): | ||
| """NV3 stack trust center token value.""" | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,7 +21,9 @@ | |
| import zigpy.state | ||
| import zigpy.types | ||
| import zigpy.util | ||
| from zigpy.zcl.clusters.greenpower import NotificationOptions, NotificationSchema | ||
| import zigpy.zdo.types as zdo_t | ||
| import zigpy.zgp.types as zgp_t | ||
|
|
||
| import bellows | ||
| from bellows.config import ( | ||
|
|
@@ -56,6 +58,7 @@ | |
| COUNTER_RX_BCAST = "broadcast_rx" | ||
| COUNTER_RX_MCAST = "multicast_rx" | ||
| COUNTER_RX_UNICAST = "unicast_rx" | ||
| COUNTER_RX_GP = "green_power_rx" | ||
| COUNTER_UNKNOWN_DEVICE = "unknown_device_rx" | ||
| COUNTER_WATCHDOG = "watchdog_reset_requests" | ||
| COUNTERS_EZSP = "ezsp_counters" | ||
|
|
@@ -75,6 +78,13 @@ | |
|
|
||
| DEFAULT_TX_POWER = 8 # dBm | ||
|
|
||
| # Zigbee Green Power spec constants — hardcoded rather than imported from | ||
| # ``zigpy.zgp`` so bellows does not depend on the upstream zigpy release | ||
| # shipping these symbols. These values are fixed by the ZGP 1.1b profile. | ||
|
puddly marked this conversation as resolved.
Outdated
|
||
| GP_ENDPOINT = 242 | ||
| GP_CLUSTER_ID = 0x0021 | ||
| GP_PROFILE_ID = 0xA1E0 | ||
|
|
||
| LIB_VERSION = importlib.metadata.version("bellows") | ||
| LOGGER = logging.getLogger(__name__) | ||
|
|
||
|
|
@@ -696,6 +706,119 @@ def ezsp_callback_handler(self, frame_name, args): | |
| self.handle_route_error(status, nwk) | ||
| elif frame_name == "idConflictHandler": | ||
| self._handle_id_conflict(*args) | ||
| elif frame_name == "gpepIncomingMessageHandler": | ||
| self._handle_gp_frame(args) | ||
|
|
||
| def _handle_gp_frame(self, args: tuple) -> None: | ||
| """Forward a decoded gpepIncomingMessageHandler callback to zigpy. | ||
|
|
||
| The NCP has already done the GP stub processing (proxy-table lookup, | ||
| duplicate suppression, etc.) and hands us a flattened GPDF. zigpy's | ||
| GP manager on the other side expects a ZigbeePacket on endpoint 242, | ||
| cluster 0x0021, carrying a ZCL GP Notification (server command 0x00) | ||
| so it can route commissioning, data, and decommissioning frames | ||
| through the same entry point. | ||
|
|
||
| We therefore rebuild that envelope here, mirroring what zigbee- | ||
| herdsman's ember adapter does. Any GPDF that cannot be converted | ||
| (for example an IEEE-addressed frame, which the current zigpy GP | ||
| stack explicitly does not support) is logged and dropped. | ||
|
puddly marked this conversation as resolved.
Outdated
|
||
| """ | ||
| if len(args) < 13: | ||
| LOGGER.debug("gpepIncomingMessageHandler: short args %r, dropping", args) | ||
| return | ||
|
|
||
| ( | ||
| _status, | ||
| gpd_link, | ||
| sequence_number, | ||
| addr, | ||
| gpdf_security_level, | ||
| gpdf_security_key_type, | ||
| _auto_commissioning, | ||
| _bidirectional_info, | ||
| gpd_security_frame_counter, | ||
| gpd_command_id, | ||
| _mic, | ||
| _proxy_table_index, | ||
| gpd_command_payload, | ||
| *rest, | ||
| ) = args | ||
|
|
||
| # v16+ appends an SlRxPacketInfo after the payload. Pull the | ||
| # proxy NWK out of it if it is there, otherwise fall back to the | ||
| # coordinator short address (the coordinator acts as the proxy). | ||
| packet_info = rest[0] if rest else None | ||
|
|
||
| if addr.applicationId == zgp_t.ApplicationID.SrcID: | ||
| source_id = addr.source_id | ||
| else: | ||
| LOGGER.debug( | ||
|
Comment on lines
+714
to
+747
|
||
| "GP frame with unsupported applicationId %s (only SrcID is " | ||
| "handled), dropping", | ||
| addr.applicationId, | ||
| ) | ||
| return | ||
|
|
||
| options = NotificationOptions( | ||
| application_id=zgp_t.ApplicationID(addr.applicationId), | ||
| also_unicast=0, | ||
| also_derived_group=0, | ||
| also_commissioned_group=0, | ||
| security_level=zgp_t.SecurityLevel(gpdf_security_level), | ||
| security_key_type=zgp_t.SecurityKeyType(gpdf_security_key_type), | ||
| appoint_temp_master=0, | ||
| tx_queue_full=0, | ||
| _reserved=0, | ||
| ) | ||
|
|
||
| notification = NotificationSchema( | ||
| options=options, | ||
| gpd_id=zgp_t.DeviceID(source_id), | ||
| frame_counter=zigpy.types.uint32_t(gpd_security_frame_counter), | ||
| command_id=zigpy.types.uint8_t(gpd_command_id), | ||
| payload=zigpy.types.LVBytes(bytes(gpd_command_payload)), | ||
| ) | ||
|
|
||
| # ZCL header: frame_control=0x01 (cluster-specific, client→server, | ||
| # not manufacturer-specific, no default response disabled), TSN is | ||
| # the GPD MAC sequence number so duplicates can be tracked, and the | ||
| # command is GP Notification (0x00). | ||
| zcl_bytes = ( | ||
| bytes([0x01, int(sequence_number) & 0xFF, 0x00]) + notification.serialize() | ||
| ) | ||
|
puddly marked this conversation as resolved.
Outdated
|
||
|
|
||
| if packet_info is not None: | ||
| proxy_nwk = int(packet_info.sender_short_id) | ||
| lqi = int(packet_info.last_hop_lqi) | ||
| rssi = int(packet_info.last_hop_rssi) | ||
| else: | ||
| proxy_nwk = int(self.state.node_info.nwk) | ||
| lqi = int(gpd_link) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this right?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the v13/v14 fallback when no SlRxPacketInfo trailer is present.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Comment added in 2df0ea5 |
||
| rssi = 0 | ||
|
|
||
| self.state.counters[COUNTERS_CTRL][COUNTER_RX_GP].increment() | ||
|
|
||
| self.packet_received( | ||
| zigpy.types.ZigbeePacket( | ||
| src=zigpy.types.AddrModeAddress( | ||
| addr_mode=zigpy.types.AddrMode.NWK, | ||
| address=zigpy.types.NWK(proxy_nwk), | ||
| ), | ||
| src_ep=zigpy.types.uint8_t(GP_ENDPOINT), | ||
| dst=zigpy.types.AddrModeAddress( | ||
| addr_mode=zigpy.types.AddrMode.NWK, | ||
| address=self.state.node_info.nwk, | ||
| ), | ||
| dst_ep=zigpy.types.uint8_t(GP_ENDPOINT), | ||
| tsn=zigpy.types.uint8_t(int(sequence_number) & 0xFF), | ||
| profile_id=zigpy.types.uint16_t(GP_PROFILE_ID), | ||
| cluster_id=zigpy.types.uint16_t(GP_CLUSTER_ID), | ||
| data=zigpy.types.SerializableBytes(zcl_bytes), | ||
| lqi=zigpy.types.uint8_t(lqi), | ||
| rssi=zigpy.types.int8s(rssi), | ||
| ) | ||
| ) | ||
|
|
||
| def _handle_frame( | ||
| self, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These verbose Claude comments are not useful, please remove them.