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

Add hwdb parser to check for inconsistencies #3906

Closed
wants to merge 8 commits into from

Conversation

keszybz
Copy link
Member

@keszybz keszybz commented Aug 5, 2016

No description provided.

This way it's clear that the property block does not end at the comment.
The python checker will complain if this is not the case.
We had a few bugs before where two match blocks were merged by mistake,
and this change should help avoid that.
This works for hwdb/[67]0-*.hwdb. I also added code to parse hwdb/20-*, but those
files are huge, and parsing them using this parser is annoyingly slow (about one
minute for the biggest files). So I removed the support for hwdb/20-*, a much simpler
hand-generated parser should suffice for those.

Current output:

hwdb/60-evdev.hwdb: 24 match groups, 35 matches, 88 properties, 0.19323015213012695s to parse
Match 'evdev:input:b0003v05ACp0259*' is duplicated
Match 'evdev:input:b0003v05ACp025A*' is duplicated
Match 'evdev:input:b0003v05ACp025B*' is duplicated
hwdb/60-keyboard.hwdb: 122 match groups, 188 matches, 638 properties, 1.0906572341918945s to parse
Failed to parse: 'KEYBOARD_KEY_8F=switchvideomode'
Failed to parse: 'KEYBOARD_KEY_C0183=media'
Failed to parse: 'KEYBOARD_KEY_C0201=new'
Failed to parse: 'KEYBOARD_KEY_C0289=reply'
Failed to parse: 'KEYBOARD_KEY_C028B=forwardmail'
Failed to parse: 'KEYBOARD_KEY_C028C=send'
Failed to parse: 'KEYBOARD_KEY_C021A=undo'
Failed to parse: 'KEYBOARD_KEY_C0279=redo'
Failed to parse: 'KEYBOARD_KEY_C0208=print'
Failed to parse: 'KEYBOARD_KEY_C0207=save'
Failed to parse: 'KEYBOARD_KEY_C0194=file'
Failed to parse: 'KEYBOARD_KEY_C01A7=documents'
Failed to parse: 'KEYBOARD_KEY_C01B6=images'
Failed to parse: 'KEYBOARD_KEY_C01B7=sound'
Property KEYBOARD_KEY_c7 is duplicated
Failed to parse: 'KEYBOARD_KEY_cF=end'
hwdb/70-mouse.hwdb: 62 match groups, 93 matches, 68 properties, 0.34186625480651855s to parse
Match 'mouse:usb:v046dpc51b:name:Logitech USB Receiver:' is duplicated
hwdb/70-pointingstick.hwdb: 5 match groups, 14 matches, 7 properties, 0.06518816947937012s to parse
hwdb/70-touchpad.hwdb: 3 match groups, 5 matches, 3 properties, 0.039690494537353516s to parse

Subsequest commits will clean those issues up.
One pattern is duplicated. It cannot apply to both mice equally, but I
have no idea which addition is more correct:

commit 75440a5
Author: Thomas H. P. Andersen <phomes@gmail.com>
Date:   Tue Aug 4 22:12:35 2015 +0200

    hwdb: add Logitech LX8 DPI and wheel click settings

commit ea24343
Author: Thomas H. P. Andersen <phomes@gmail.com>
Date:   Tue Sep 1 22:01:22 2015 +0200

    hwdb: more mice

I dropped the later, since it added multiple matches so an error is more
likely.
It's hard to say if those were supposed to be different patterns…
It seems awkward to have both cases mixes. Note that the real parser
accepts both cases, and this only standarizes the usage in the systemd
database.
It's hard to say which one of the two mappings should stay. But the later
one would win (when both very present), and nobody complained, so let's
assume that that's the one.
@@ -76,9 +76,6 @@ evdev:input:b0003v05ACp0254*
EVDEV_ABS_36=::92

# MacbookPro10,1 (unibody, June 2012)
evdev:input:b0003v05ACp0259*
evdev:input:b0003v05ACp025A*
evdev:input:b0003v05ACp025B*
Copy link
Member Author

Choose a reason for hiding this comment

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

@whot You added this one, please have a look.

Copy link
Contributor

Choose a reason for hiding this comment

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

these should be fixed (range is 0262, 0263 and 0264), it's a simple copy-paste error. I sent a PR to fix these here: #3921

@poettering
Copy link
Member

lgtm. But maybe @whot and @martinpitt should bless this too! @whot? @martinpitt?

whot added a commit to whot/systemd that referenced this pull request Aug 8, 2016
Copy-paste error, correct IDs from the kernel's drivers/input/mouse/bcm5974.c

Fixes: systemd#3906
poettering pushed a commit that referenced this pull request Aug 8, 2016
Copy-paste error, correct IDs from the kernel's drivers/input/mouse/bcm5974.c

Fixes: #3906
keszybz added a commit to keszybz/systemd that referenced this pull request Aug 9, 2016
Quoting systemd#3906 (comment):
> According to
> http://support.logitech.com/en_us/product/v220-cordless-optical-mouse-for-notebooks
> it seems the mouse is using a pre-version of the small unifying receiver we
> know now. If there are 2 mice with the same receiver, that means that the
> values should both be dropped IMO.
@keszybz
Copy link
Member Author

keszybz commented Aug 9, 2016

PR reopened, I still hope to run those tests automatically.

I removed the commit for Apple touchpads, since it was resolved in #3291, and dropped both entries for the unifying receiver as suggested by @bentiss.

@keszybz
Copy link
Member Author

keszybz commented Aug 9, 2016

Hm, it seems that github does not allow reopening PRs. I'll make a new one.

edolstra pushed a commit to NixOS/systemd that referenced this pull request Sep 30, 2016
Copy-paste error, correct IDs from the kernel's drivers/input/mouse/bcm5974.c

Fixes: systemd#3906
(cherry picked from commit 9c06792)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

4 participants