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

Restore the GattData::process public API #311

Merged
merged 1 commit into from
Mar 6, 2025

Conversation

ivmarkov
Copy link
Collaborator

@ivmarkov ivmarkov commented Mar 6, 2025

As discussed in the Embassy Matrix chat, having a public GattData::process API allows for "mixed-processing" scenarios, where the ATT PDUs are pulled using the "raw" ATT Connection type - yet - some of those PDUs are delegated to (an) attribute server via GattData::process, while others are processed at the ATT PDU level.

This is precisely what the Matter BTP implementation over trouble does here.

(This API was retired by GIT commit cb8985e#diff-27c58f7be9fbba9f7582453b30b6cde8d67e41414fe9004daea290d4c87da375 and I'm just restoring it here, except based on DynamicAttributeServer now, as that's what the new GattConnection has and needs.)

===

Additionally, I've addressed two hygienic issues in the #[gatt_server] proc-macro, where an unqualified Error type was used which was clashing with whatever the user had imported in terms of Error and which furthermore required the user to explicitly import trouble_host::Error which is NOK.

===

Will un-draft once I test it a bit and confirm it works.

@ivmarkov ivmarkov force-pushed the restore-gattdata-process branch from 5d716e2 to 22dd85b Compare March 6, 2025 09:40
@ivmarkov ivmarkov force-pushed the restore-gattdata-process branch from 22dd85b to 19a68aa Compare March 6, 2025 09:48
Copy link
Member

@lulf lulf left a comment

Choose a reason for hiding this comment

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

Thanks for fixing!

@ivmarkov ivmarkov marked this pull request as ready for review March 6, 2025 12:16
@ivmarkov
Copy link
Collaborator Author

ivmarkov commented Mar 6, 2025

/test

@ivmarkov
Copy link
Collaborator Author

ivmarkov commented Mar 6, 2025

@lulf Seems me issuing /test does not work?

@lulf
Copy link
Member

lulf commented Mar 6, 2025

/test

@lulf
Copy link
Member

lulf commented Mar 6, 2025

@ivmarkov Since this is running on my personal hardware, the command will only work for members with write access. I've sent you an invite now :)

@lulf lulf merged commit 8861159 into embassy-rs:main Mar 6, 2025
3 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