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

Clean up home key in MRP implementation. #696

Closed
wants to merge 1 commit into from

Conversation

ckeehan
Copy link
Contributor

@ckeehan ckeehan commented Jun 5, 2020

The HID key the Remote app sends for home and home_hold is different than what is used in the MRP implementation here. Functionality remains the same just updating to send the same key the Remote app does.

The HID key the Remote app sends for home and home_hold is different than what is used in the MRP implementation here. Functionality remains the same here.
@postlund
Copy link
Owner

postlund commented Jun 5, 2020

Looks good! You need to make the corresponding update to the MRP service in the fake device since the tests fails. I would also like it if you make it a bit more clear in the commit message what you changed, I usually look at them when writing the change log so it helps a lot. If possible, I try to prefix with a submodule name or something to easier group changes, e.g. "mrp: Correct home button HID code" or something.

@postlund postlund added the mrp Media Remote Protocol label Jun 5, 2020
@postlund postlund added this to the v0.7.0 milestone Jun 5, 2020
@postlund
Copy link
Owner

Just wanted to see if you want to finish off this PR or if I should take over and get it done?

@postlund
Copy link
Owner

postlund commented Jul 8, 2020

Now I'm starting to wonder that is correct once again regarding this... At the moment top_menu sends the same HID-code as you change home to. I will have to re-evaluate this again to make it right.

@postlund
Copy link
Owner

I don't think this is relevant anymore.

@postlund postlund closed this Jun 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mrp Media Remote Protocol
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants