Skip to content

Commit

Permalink
Fix PostAttributeChangeCallback value typing (#36928)
Browse files Browse the repository at this point in the history
As documented in the TODO, the callback requires the caller to pass a
NULL-terminated C-string, but it’s actually a length-buffer pair.
The python lighting example receives values with incorrect length due to this
issue.

This PR fixes the FFI interface by wrapping the callback function, and converts
the length-buffer pair to a proper python bytes value.
  • Loading branch information
sasdf authored Jan 21, 2025
1 parent cc14a69 commit c2e655c
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 10 deletions.
25 changes: 18 additions & 7 deletions src/controller/python/chip/native/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

import ctypes
import enum
import functools
import glob
import os
import platform
Expand Down Expand Up @@ -146,21 +147,31 @@ def __ne__(self, other):
return not self == other


PostAttributeChangeCallback = ctypes.CFUNCTYPE(
c_PostAttributeChangeCallback = ctypes.CFUNCTYPE(
None,
ctypes.c_uint16,
ctypes.c_uint16,
ctypes.c_uint16,
ctypes.c_uint8,
ctypes.c_uint16,
# TODO: This should be a pointer to uint8_t, but ctypes does not provide
# such a type. The best approximation is c_char_p, however, this
# requires the caller to pass NULL-terminate C-string which might
# not be the case here.
ctypes.c_char_p,
ctypes.POINTER(ctypes.c_char),
)


def PostAttributeChangeCallback(func):
@functools.wraps(func)
def wrapper(
endpoint: int,
clusterId: int,
attributeId: int,
xx_type: int,
size: int,
value: ctypes.POINTER(ctypes.c_char),
):
return func(endpoint, clusterId, attributeId, xx_type, size, value[:size])
return c_PostAttributeChangeCallback(wrapper)


def FindNativeLibraryPath(library: Library) -> str:
"""Find the native CHIP dll/so path."""

Expand Down Expand Up @@ -234,7 +245,7 @@ def _GetLibraryHandle(lib: Library, expectAlreadyInitialized: bool) -> _Handle:
[ctypes.POINTER(PyChipError), ctypes.c_char_p, ctypes.c_uint32])
elif lib == Library.SERVER:
setter.Set("pychip_server_native_init", PyChipError, [])
setter.Set("pychip_server_set_callbacks", None, [PostAttributeChangeCallback])
setter.Set("pychip_server_set_callbacks", None, [c_PostAttributeChangeCallback])

handle = _nativeLibraryHandles[lib]
if expectAlreadyInitialized and not handle.initialized:
Expand Down
15 changes: 12 additions & 3 deletions src/controller/python/chip/server/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,20 @@
import ctypes

from chip import native
from chip.native import PostAttributeChangeCallback
from chip.native import PostAttributeChangeCallback, c_PostAttributeChangeCallback

__all__ = [
"GetLibraryHandle",
"PostAttributeChangeCallback",
]

def GetLibraryHandle(cb: PostAttributeChangeCallback) -> ctypes.CDLL:
"""Get a memoized handle to the chip native code dll."""

def GetLibraryHandle(cb: c_PostAttributeChangeCallback) -> ctypes.CDLL:
"""Get a memoized handle to the chip native code dll.
Args:
cb: A callback decorated by PostAttributeChangeCallback decorator.
"""

handle = native._GetLibraryHandle(native.Library.SERVER, False)
if not handle.initialized:
Expand Down

0 comments on commit c2e655c

Please sign in to comment.