Skip to content

Commit

Permalink
Fix PostAttributeChangeCallback value typing
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 committed Dec 22, 2024
1 parent af336ec commit 185e1bb
Show file tree
Hide file tree
Showing 2 changed files with 25 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
10 changes: 7 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,15 @@
import ctypes

from chip import native
from chip.native import PostAttributeChangeCallback
from chip.native import c_PostAttributeChangeCallback, 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 185e1bb

Please sign in to comment.