Dataclass-based ASCS Packets

[zxzxwu]

This has been used in Navi for a while. Recently I resolved several critical issues and I think it's a time to bring it into Bumble upstream.

An obvious benefit to use dataclasses is IDE and type checker integration.

Unlike #430, this design properly utilize the existing effort, especially field specs. For nested fields, we may need to set another field in metadata like Annotated[list, 1, '[' or ']' ] as stack operator, though usually we only support up to 2 layers.

return_parameters_fields cannot be benifit from this, because current type checker cannot associate two classes. But BTW, return_parameters_fields is actually not necessary to be inside decorator, so later we may have a PR to move them from decorators to classes.

[zxzxwu]

I believe there are still a lot of places to polish, so this is just an initial draft for reviews.

[barbibulle]

I like the idea. Having support for proper typechecking via dataclasses is a great way to go (easier that designing type-checking plugins). We'll need to make sure that we keep things still compatible with Python >= 3.9.

[zxzxwu]

Thanks! typing.Annotated is introduced in Python 3.9, so compatability shouldn't be a problem (and even under 3.9 we can import it from typing_extensions). But of course we will need some test cases to handle them.

I will work on #686 first, and them continue here.

[zxzxwu]

A problem here is that with from __future__ import annotations, Annotations including metadata will become string in Runtime. Evaluating them from HCI module is not a good idea because it's not always possible to get the symbols.

There are 2 solutions to do this:

1. Avoid using from __future__ import annotations when dataclass-based packets are used. It seems PEP 563 will never become mandatory (replaced by PEP 649), so it should be safe, but it requires some rework to remove this import.
2. Using dataclasses.field(metadata={...}) to store field info, but this will be a little bit long-winded.

[zxzxwu]

This PR is ready for review. Let's get started from ASCS packets since the test is more comprehensive here.

[barbibulle]

This PR is ready for review. Let's get started from ASCS packets since the test is more comprehensive here.

I'll take a look shortly. I need to refresh my memory on deferred type annotation evaluation.

[barbibulle]

I think there may be a slight variant on this model that may be a bit simpler, a bit easier to read, and would still let us use from __future__ import annotations. Instead of using Annotated to change the type annotation, we can usedataclasses.field, via a helper function. In a first step, we can keep the existing 'spec' syntax exactly the way it is (makes it easy to just convert 1:1 to the new scheme without having to worry about compatibility). The only change being how list of fields are represented.
I'm thinking of something like this:

def hci_field(spec, **kw_args):
    return dataclasses.field(
        metadata={"bumble.hci": {"spec": spec, "options": kw_args}}
    )

@dataclasses.dataclass
class Bar:
    # equivalent of:
    #  ('my_field1', 1)
    my_field1: int = hci_field(1)

    # equivalent of:
    #  ('my_field2', '*')
    my_field2: bytes = hci_field("*")

    # equivalent of:
    #  ('my_field3', Address.parse_address)
    my_field3: Address = hci_field(Address.parse_address)

    # equivalent of:
    #  ('my_field4', STATUS_SPEC)
    my_field4: int = hci_field(STATUS_SPEC)

    # equivalent of:
    # [
    #   ('my_field5', 1),
    #   ('my_field6', 1)
    #   ('my_field7', 1)
    # ]
    my_field5: list[int] = hci_field(1, start_list=True)
    my_field6: list[int] = hci_field(1)
    my_field7: list[int] = hci_field(1, end_list=True)


bar = Bar(
    my_field1=7,
    my_field2=b"hello",
    my_field3=Address("AA:BB:CC:DD:EE:FF"),
    my_field4=55,
    my_field5=[1],
    my_field6=[2],
    my_field7=[3],
)
print(dataclasses.fields(bar))