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

Implement Hap support #532

Merged
merged 34 commits into from
Sep 9, 2024
Merged

Implement Hap support #532

merged 34 commits into from
Sep 9, 2024

Conversation

wescande
Copy link
Contributor

Hearing Access Service 1.0 implementation

bumble/profiles/hap.py Show resolved Hide resolved
bumble/profiles/hap.py Outdated Show resolved Hide resolved
bumble/profiles/hap.py Outdated Show resolved Hide resolved
bumble/profiles/hap.py Outdated Show resolved Hide resolved
bumble/profiles/hap.py Outdated Show resolved Hide resolved
bumble/profiles/hap.py Outdated Show resolved Hide resolved
raise error instead of assert
use __bytes__
use Union instead of foo | bar
use dataclass
use spawn to send read preset response after ATT_WRITE_RSP

use public_address to keep info about bonded devices (vs connection)

use watcher to be notified of connection / disconnection

Fix next_preset logic when selecting the previous preset

Use indicate for preset changed operation
bumble/profiles/hap.py Outdated Show resolved Hide resolved
bumble/profiles/hap.py Outdated Show resolved Hide resolved
bumble/profiles/hap.py Outdated Show resolved Hide resolved
bumble/profiles/hap.py Outdated Show resolved Hide resolved
bumble/profiles/hap.py Outdated Show resolved Hide resolved
bumble/profiles/hap.py Outdated Show resolved Hide resolved
@wescande
Copy link
Contributor Author

In order to check if this is working, I am currently trying to implement an HAP test in avatar
See https://android-review.git.corp.google.com/q/topic:%22hap_for_charlie%22 that is still work in progress as of today.

I am struggling with having a connection as Android require LeAudio to activate Hap :/

@zxzxwu
Copy link
Collaborator

zxzxwu commented Aug 20, 2024

In order to check if this is working, I am currently trying to implement an HAP test in avatar See https://android-review.git.corp.google.com/q/topic:%22hap_for_charlie%22 that is still work in progress as of today.

I am struggling with having a connection as Android require LeAudio to activate Hap :/

Our experience is that most developers prefer to use a standalone app than RPC console or a test script, as it's easier to observe the behavior for debugging. Also, for Bumble contribution, it's better to have an example teaching people how to use this libraries. You can try to copy Unicast examples and integrate it with HAP.

@zxzxwu zxzxwu closed this Aug 20, 2024
@zxzxwu zxzxwu reopened this Aug 20, 2024
@wescande
Copy link
Contributor Author

I have added a run_hap_server but I don't really understand yet how you are making use of these script.

@wescande wescande requested a review from zxzxwu August 23, 2024 21:02
Copy link
Collaborator

@zxzxwu zxzxwu left a comment

Choose a reason for hiding this comment

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

Could you also provide unit tests? Thanks!

bumble/profiles/hap.py Outdated Show resolved Hide resolved
bumble/profiles/hap.py Outdated Show resolved Hide resolved
bumble/profiles/hap.py Outdated Show resolved Hide resolved
bumble/profiles/hap.py Show resolved Hide resolved
bumble/profiles/hap.py Outdated Show resolved Hide resolved
bumble/profiles/hap.py Outdated Show resolved Hide resolved
bumble/profiles/hap.py Outdated Show resolved Hide resolved
@wescande
Copy link
Contributor Author

Could you also provide unit tests? Thanks!

There is no client implemented yet. I am not sure how to test it without and I haven't plan implementing the client part for now

@zxzxwu
Copy link
Collaborator

zxzxwu commented Aug 28, 2024

Could you also provide unit tests? Thanks!

There is no client implemented yet. I am not sure how to test it without and I haven't plan implementing the client part for now

We can have a very brief client just for testing like VCP:

async def _on_set_next_preset(self, connection: Optional[Connection], __value__: bytes) -> None:
await self.set_next_or_previous_preset(connection, False)

async def _on_set_previous_preset(self, connection: Optional[Connection], __value__: bytes) -> None:
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is the purpose of using __value__? An I cannot find what does it mean online

@zxzxwu
Copy link
Collaborator

zxzxwu commented Aug 29, 2024

Approved as a milestone. I can merge it when the format/lint errors are fixed.

Still, there are still too many TODOs and loud logs. I hope in following PRs we can stabilize most of them.

@SilverBzH SilverBzH merged commit 00e660d into google:main Sep 9, 2024
41 of 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.

3 participants