Skip to content
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

Add a class-based GATT adapter #598

Merged
merged 1 commit into from
Dec 3, 2024
Merged

Add a class-based GATT adapter #598

merged 1 commit into from
Dec 3, 2024

Conversation

barbibulle
Copy link
Collaborator

This new adapter makes it convenient to use classes that follow the from_bytes and __bytes__ conventions. For any class that conforms to this protocol, the adapter will automatically convert GATT bytes values to/from class objects.

This PR also removes the support for directly constructing a gatt.Characteristic with an str value. Instead, callers must either perform the conversion themselves, or use the standard UT8CharacteristicAdapter. (allowing passing str at construction was a broken API, because it would perform the conversion to bytes only at construction time, but wouldn't perform the conversion subsequently if assigning a new value. So it is better to remove it altogether.

@barbibulle barbibulle requested a review from zxzxwu November 23, 2024 17:18
@@ -759,13 +759,13 @@ class AttributeValue:
def __init__(
self,
read: Union[
Callable[[Optional[Connection]], bytes],
Callable[[Optional[Connection]], Awaitable[bytes]],
Callable[[Optional[Connection]], Any],
Copy link
Collaborator

Choose a reason for hiding this comment

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

I hope this can be a TypeVar, but it might always require a type without PEP 696 which is annoying.

@@ -735,6 +743,24 @@ def decode_value(self, value: bytes) -> str:
return value.decode('utf-8')


# -----------------------------------------------------------------------------
class SerializableCharacteristicAdapter(CharacteristicAdapter):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it possible to solve the problem that CharacteristicAdapter is not considered as a Characteristic?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, I think it's possible to do that, and it would be nice to improve type checking. Currently, the adapter classes are bit hard to deal with when it comes to types, because they are like dynamic decorators, they are polymorphic at runtime (that's convenient to reuse the code, but not convenient for typing). I have an idea about how to fix this, by splitting the adapter classes into separate classes for attributes and attribute proxies. That should solve the problem. But that's a larger change, so I'd like to do that in a separate PR.

@barbibulle barbibulle merged commit 3ce7b92 into main Dec 3, 2024
57 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