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

Bumble hid #295

Merged
merged 16 commits into from
Oct 20, 2023
Merged

Bumble hid #295

merged 16 commits into from
Oct 20, 2023

Conversation

SneKarnataki
Copy link
Contributor

Bumble HID Host implementation.

rdhavan and others added 6 commits September 15, 2023 06:45
Includes:
1. HID Host implementation - hid.py
2. HID application to test Host with 3rd party HID Device application - run_hid_host.py
3. HID supporting files for testing - hid_report_parser.py & hid_key_map.py

Commands to run the application:
Default application:
python run_hid_host.py classic1.json usb:0 <device bd-addr>

Menu options for testing (Get/Set):
python run_hid_host.py classic1.json usb:0 <device bd-addr> test-mode

CuttleFish:tcp-client:127.0.0.1:7300

Application used for testing as Device : Bluetooth Keyboard & Mouse-5.3.0.apk

Note: Change in sdp.py file while testing hid profile,
TEXT_STRING: lambda x: DataElement(DataElement.TEXT_STRING, x.decode('utf8')) changed to
TEXT_STRING: lambda x: DataElement(DataElement.TEXT_STRING, x)
as we were facing error "UnicodeDecodeError: 'utf-8' codec can't decode byte 0xa1 in position 4: invalid start byte" while fetching sdp records.
TEXT_STRING: lambda x: DataElement(DataElement.TEXT_STRING, x.decode('utf8')) changed to
TEXT_STRING: lambda x: DataElement(DataElement.TEXT_STRING, x)
as we were facing error "UnicodeDecodeError: 'utf-8' codec can't decode byte 0xa1 in position 4: invalid start byte" while fetching sdp records.
bumble/hid.py Show resolved Hide resolved
bumble/hid.py Outdated Show resolved Hide resolved
bumble/hid.py Outdated Show resolved Hide resolved
bumble/hid.py Outdated Show resolved Hide resolved
bumble/hid.py Outdated Show resolved Hide resolved
bumble/hid.py Outdated Show resolved Hide resolved
bumble/hid.py Outdated Show resolved Hide resolved
bumble/hid.py Outdated Show resolved Hide resolved
bumble/sdp.py Show resolved Hide resolved
bumble/hid.py Outdated Show resolved Hide resolved
bumble/hid.py Outdated Show resolved Hide resolved
bumble/hid.py Outdated Show resolved Hide resolved
examples/run_hid_host.py Outdated Show resolved Hide resolved
bumble/hid.py Outdated Show resolved Hide resolved
bumble/hid.py Outdated Show resolved Hide resolved
@SneKarnataki
Copy link
Contributor Author

SneKarnataki commented Sep 27, 2023

Hi zxzxwu, barbibulle,

Thank you for your comment. We have checked-in the updated files in bumble_hid branch.
Request you to provide any feedback.

Regards,
LTIMindtree Team

bumble/hid.py Outdated Show resolved Hide resolved
bumble/hid.py Outdated Show resolved Hide resolved
bumble/hid.py Outdated Show resolved Hide resolved
bumble/hid.py Outdated Show resolved Hide resolved
bumble/sdp.py Outdated Show resolved Hide resolved
bumble/sdp.py Outdated Show resolved Hide resolved
bumble/hid.py Outdated Show resolved Hide resolved
bumble/hid.py Outdated Show resolved Hide resolved
bumble/hid.py Outdated Show resolved Hide resolved
bumble/hid.py Outdated Show resolved Hide resolved
bumble/hid.py Outdated Show resolved Hide resolved
bumble/hid.py Outdated Show resolved Hide resolved
bumble/hid.py Outdated Show resolved Hide resolved
@SneKarnataki
Copy link
Contributor Author

Hi all,

We have updated review comments.
Thank you.

Regards,
LTIMindtree Team

bumble/hid.py Outdated Show resolved Hide resolved
bumble/hid.py Show resolved Hide resolved
bumble/hid.py Outdated Show resolved Hide resolved
bumble/hid.py Show resolved Hide resolved
examples/run_hid_host.py Outdated Show resolved Hide resolved
@SneKarnataki
Copy link
Contributor Author

Hi barbibulle,
Thank you for inputs.
We have updated the file.

Regards,
LTIMindtree team

@barbibulle
Copy link
Collaborator

Hi barbibulle, Thank you for inputs. We have updated the file.

Regards, LTIMindtree team

This looks good, thanks. Please see a couple new (minor) comments I added as I was reading through the code again.

Also, please run the formatter before this can be merged (see the output of the GitHub actions to see the pre-commit checks that failed). You can run the formatter by running invoke project.format from the top level (you'll need to `pip install -e ".[development]" to ensure you have the proper linter and formatter installed so you can run the pre-commit checks including the linter and formatter.

Thanks.

@SneKarnataki
Copy link
Contributor Author

Hi barbibulle, Thank you for inputs. We have updated the file.
Regards, LTIMindtree team

This looks good, thanks. Please see a couple new (minor) comments I added as I was reading through the code again.

Also, please run the formatter before this can be merged (see the output of the GitHub actions to see the pre-commit checks that failed). You can run the formatter by running invoke project.format from the top level (you'll need to `pip install -e ".[development]" to ensure you have the proper linter and formatter installed so you can run the pre-commit checks including the linter and formatter.

Thanks.

Thank you barbibulle. I was able to perform "invoke project.format". But I am unable to view "Please see a couple new (minor) comments I added as I was reading through the code again." comments in PR.

bumble/hid.py Show resolved Hide resolved
bumble/hid.py Outdated Show resolved Hide resolved
@barbibulle
Copy link
Collaborator

Hi barbibulle, Thank you for inputs. We have updated the file.
Regards, LTIMindtree team

This looks good, thanks. Please see a couple new (minor) comments I added as I was reading through the code again.
Also, please run the formatter before this can be merged (see the output of the GitHub actions to see the pre-commit checks that failed). You can run the formatter by running invoke project.format from the top level (you'll need to `pip install -e ".[development]" to ensure you have the proper linter and formatter installed so you can run the pre-commit checks including the linter and formatter.
Thanks.

Thank you barbibulle. I was able to perform "invoke project.format". But I am unable to view "Please see a couple new (minor) comments I added as I was reading through the code again." comments in PR.

Sorry about that, the two comments were pending and I forgot to send them. They should be visible now.

Copy link
Contributor

@uael uael left a comment

Choose a reason for hiding this comment

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

Good job 🎉

bumble/hid.py Outdated Show resolved Hide resolved
bumble/hid.py Outdated Show resolved Hide resolved
bumble/hid.py Outdated Show resolved Hide resolved
bumble/hid.py Outdated Show resolved Hide resolved
@barbibulle
Copy link
Collaborator

@SneKarnataki the "Code format and lint check" GitHub action still fails on this PR (a couple places where the typing needs to be fixed). Can you check the output of the GitHub action and fix the issue before we can merge?
(I recommend running invoke project.pre-commit locally before pushing commits, this way you can catch most of these error in advance).

@uael
Copy link
Contributor

uael commented Oct 16, 2023

One more format issue 🙏

diff --git a/examples/hid_report_parser.py b/examples/hid_report_parser.py
index e5f407f..61561b3 100644
--- a/examples/hid_report_parser.py
+++ b/examples/hid_report_parser.py
@@ -139,7 +139,6 @@ class Mouse:
 
 # ------------------------------------------------------------------------------
 class ReportParser:
-
     @staticmethod
     def parse_input_report(input_report: bytes) -> None:

@SneKarnataki
Copy link
Contributor Author

Thank you all for guiding our first GitHub checkin and helping to make it optimized.

Regards,
LTIMindtree

@uael uael merged commit 78b8b50 into google:main Oct 20, 2023
51 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.

6 participants