search

mwyborski / Linux-Magic-Trackpad-2-Driver

499 stars 84 forks source link

Mainlining code? #19

Closed rohitpid closed 5 years ago

rohitpid commented 6 years ago

Have you started the process to mainline this? I have code for the magic mouse 2 that I'd also like to mainline sometime. I'm still working on improving it but it'd be nice to know what kind of timeline you were thinking of. I could do that separately after yours to keep the patches smaller or would it be better to create a PR for this repo?

mwyborski commented 6 years ago

Hi @rohitpid , i haven't send in the patch. Maybe it would be better to separate the two drivers, because i can't test the magic mouse 2, so i would send in a patch, that i can't even test. But i don't want to slow you down. We could make a branch for the magic mouse 2 or you fork and you keep on working on base of this driver. I don't know when i will find the time to review it and send it in, but i hope to do it soon. I have never committed something to the kernel and i don't know how complicated this will be, so i want to be able to say, that the driver is tested.

Keep up the good work, i think a lot of users are waiting for the magic mouse 2, and what i can do to support you, i will do ;-)

rohitpid commented 6 years ago

Hey @robotrovsky,

Thanks for the response and support! I do have a fork which has my changes and I rebase off of your changes periodically. I have mm1, mm2, trackpad1 and trackpad2 handy to test :-).

I figured out the bytes that are sent and received to get battery percentage reports from the mouse. Do you know how we should set this? Magic Mouse 1 and Magic Trackpad 1 report the battery percentages already but I cannot see where in hid-magicmouse.c any of it is reported. Perhaps it's in another module? Do you have any ideas or know where this is done?

rohitpid commented 6 years ago

So using a packet logger on mac, it seems that magic mouse 2 and magic trackpad 2 send bytes [41 f0] to the mouse and then receive [a1 f0 90 00 62] responses. The 62 at the end directly reports the 98% charge value of my mouse (0x62 hex = 98 dec). Looks like the way to report this out is through power_supply and power_supply_desc objects but if anyone has an example of how to do this, I'd appreciate it.

For reference magic mouse 1 sends [43 47] and gets back [a3 47 14] (the 14 correctly corresponds to 20% battery)

The weird thing is, the magic mouse 1 creates a node in /sys/class/power_supply/hid-67:d9:3f:67:56:c4-battery/capacity but the code in hid-magicmouse doesn't have any power_supply structs.

bobbysue commented 6 years ago

If you are interested, I could prepare a patch to submit for mainline review. I would need to know who to include for "signed-off-by" lines.

mwyborski commented 6 years ago

Hi @bobbysue, exucse the late answer. I am on vacation right now. I should have submitted the patch time ago. So if you would like to do this, i would appreciate it. Please make sure, that you check the patch with checkpatch.pl and that we are not changing the behaviour of the original driver for the MM and MT1. Please include @ponyfleisch and me in the signed-off-by and CC us, when you send in the patch:

"Claudio Mettler" claudio@ponyfleisch.ch https://github.com/ponyfleisch/hid-magictrackpad2 me: "Marek Wyborski" marek.wyborski@emwesoft.com

Thank you!!

mwyborski commented 6 years ago

@bobbysue maybe you have the time to look into these two issues, which seem to have the same problem, before you submit the patch:

https://github.com/robotrovsky/Linux-Magic-Trackpad-2-Driver/issues/11 https://github.com/robotrovsky/Linux-Magic-Trackpad-2-Driver/issues/23

rohitpid commented 6 years ago

@robotrovsky @bobbysue I have magic mouse 2 code that I want to submit too. Would you like me to submit it all together so that we get functionality for both?

bobbysue commented 6 years ago

@robotrovsky I haven't been able to reproduce the bugs. I've also been testing on kernel v4.18, but using chromiumOS (so no xf86)

@rohitpid I think it makes sense to submit the changes separately to reduce the complexity of each patch.

mwyborski commented 5 years ago

@bobbysue Thank you for mainlining!!! That was great!

I close this now and update the readme.

mwyborski commented 5 years ago

@bobbysue dont take all the credit for google: https://www.phoronix.com/scan.php?page=news_item&px=Linux-Apple-Magic-Trackpad-2

bobbysue commented 5 years ago

Thank you! I hadn't seen that article- it looks like they have issued an update giving you and Claudio credit, though the main text still seems off... AFAIK nobody else involved, including the kernel maintainers who gave comments, work for Google/ChromeOS...

FYI: There will probably need to be adjustments to userspace programs for the low pressure levels from the trackpad. We had an offset to make it higher, but the maintainers objected to having it in the driver, saying there should be userspace quirks for it.

mwyborski commented 5 years ago

@bobbysue no problem. It was just funny to read "Thanks Google: Linux Kernel Finally Nearing Support For The Apple Magic Trackpad 2" and then seeing that they actually talk about this driver. What actually would be really nice, would not be google working on the driver but apple releasing the spec to do the driver the right way. But i doubt this will happen.

The problem is that the pressures are not correct at all. Only for the first finger, from what i saw.

Dygear commented 5 years ago

If they gave the spec out it would also be giving away the magic. Also thank you for your work on this. My trackpad is now useable with Ubuntu! Your a super star!

— Thank you for your time, Mark “Dygear” Tomlin.

On Oct 5, 2018, at 14:57, robotrovsky notifications@github.com wrote:

Closed #19.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.