cvuchener / hidpp

Collection of HID++ tools
GNU General Public License v3.0
90 stars 23 forks source link

Fixes a bug that is seen after deactivating ModeSwitch #4

Closed pnrao closed 7 years ago

pnrao commented 7 years ago

I have a Logitech G102 mouse. In my previous configuration, I had used LGS under Windows 10 to assign one of the side buttons to G-shift ("ModeSwitch" according to ProfileFormat.cpp). I got rid of my customization under LGS and assigned all the buttons to their default roles. Apparently, the on-board memory of the mouse remembers the previously assigned G-shifted roles of the buttons. As a result of that, running sudo ./hidpp-persistent-profiles -vinfo /dev/hidraw1 read lgs-default.xml would fail with the following message:

[info:feature] Feature [0x8100] OnboardProfiles has index 0x0f
[info:feature] Feature [0x8100] OnboardProfiles has index 0x0f
terminate called after throwing an instance of 'std::runtime_error'
  what():  value is not in enum
zsh: abort      sudo ./hidpp-persistent-profiles -vinfo /dev/hidraw1 read lgs-default.xml

With my patch, the program runs without failing, and shows the following XML:

<profiles>
    <profile>
        <dir_unknown>0</dir_unknown>
        <enabled>true</enabled>
        <modes>
            <mode>
                <dpi>400</dpi>
            </mode>
            <mode>
                <dpi>800</dpi>
            </mode>
            <mode>
                <dpi>1000</dpi>
            </mode>
            <mode>
                <dpi>1600</dpi>
            </mode>
            <mode>
                <dpi>2500</dpi>
            </mode>
        </modes>
        <angle_snapping>false</angle_snapping>
        <color>ffffff</color>
        <default_dpi>2</default_dpi>
        <logo_effect>
            <brightness>49</brightness>
            <period>12000</period>
            <type>Cycle</type>
        </logo_effect>
        <name>Profile 1</name>
        <report_rate>1</report_rate>
        <revision>71</revision>
        <side_effect>
            <brightness>49</brightness>
            <period>12000</period>
            <type>Cycle</type>
        </side_effect>
        <switched_dpi>0</switched_dpi>
        <buttons>
            <mouse-button>0</mouse-button>
            <mouse-button>1</mouse-button>
            <mouse-button>2</mouse-button>
            <mouse-button>3</mouse-button>
            <mouse-button>4</mouse-button>
            <special>ResolutionCycle</special>
            <mouse-button>0</mouse-button>
            <mouse-button>1</mouse-button>
            <mouse-button>2</mouse-button>
            <special>DeactivatedSpecial</special>
            <key modifiers="LeftControl">0</key>
            <key modifiers="LeftMeta">E</key>
        </buttons>
    </profile>
</profiles>

This new configuration can be written to the mouse and read back again without errors.

cvuchener commented 7 years ago

The "DeactivatedSpecial" button is the shifted mapping for the shifting button, right? On my G502, LGS uses the disabled button type in this case. It is best to have both if LGS uses both.

No need for a "Special" suffix, I would prefer just "Deactivated", it is already in the special enum.

But I really need to catch that exception, I got it myself a few times. But I am not sure what to do with it. Maybe set the button to disabled and log an error. Or make my enum type less strict.

Your profile also remind me that there is problem with numerical keys: "0" could be no key (HID usage 0) or the "0" key on QWERTY US keyboards (HID usage 0x27).

pnrao commented 7 years ago

The "DeactivatedSpecial" button is the shifted mapping for the shifting button, right?

Yes, I had previously assigned my thumb Back button as the G-shift modifier, and the button corresponds to mouse-button 3 in the XML. Here is the snippet from my previous configuration:

    <buttons>
        <mouse-button>0</mouse-button>
        <mouse-button>1</mouse-button>
        <mouse-button>2</mouse-button>
        <special>ModeSwitch</special>
        <key modifiers="LeftControl">C</key>
        <special>ResolutionDefault</special>
        <mouse-button>0</mouse-button>
        <mouse-button>1</mouse-button>
        <mouse-button>2</mouse-button>
        <disabled/>
        <key modifiers="LeftControl">0</key>
        <key modifiers="LeftMeta">E</key>
    </buttons>

Or make my enum type less strict.

I prefer this solution because it will make your tools future-proof to some extent. An unknown value like 123 could be assigned to the string Unknown123 in the XML. That way, reading an unexpected number will not cause a problem. It would also make way for an easy-to-implement mechanism so that a configuration XML that has been generated from a device can be written back to the same device without throwing errors. I had done something like that for debugging... but it was meant as a dirty hack to locate the problem. I had not written code to handle text like Unknown123 in the XML and write back 123 to the device.

there is problem with numerical keys: "0" could be no key (HID usage 0) or the "0" key on QWERTY US keyboards

I am not sure I follow... there are three kinds of "0" already... but they are surrounded by <mouse-button> or <special> or <key> tags in the XML. I don't see a conflict here. So the QWERTY keyboard "0" would have it within <key></key> tags.

pnrao commented 7 years ago

On my G502, LGS uses the disabled button type in this case. It is best to have both if LGS uses both.

I had missed the importance of this sentence earlier. I believe G102 also behaves the same way: <disabled/> gets assigned to a button as its alternate function if you set that button for ModeSwitch. This can be seen in the snippet of my old configuration that I have pasted above. I believe that G502 and G102 should behave identically in this case. I read in your README.md that "only format 2 was tested with a G502 spectrum" and "macro format 1". Here is my output for G102:

% sudo ./hidpp20-onboard-profiles-get-description /dev/hidraw1
Memory model:   1
Profile format: 2
Macro format:   1
Profile count:  1
Profile count OOB:  1
Button count:   6
Sector count:   7
Sector size:    256
Mechanical layout:  0xa (G-shift, DPI shift)
Various info:   0x1 (Corded)
cvuchener commented 7 years ago

It would also make way for an easy-to-implement mechanism so that a configuration XML that has been generated from a device can be written back to the same device without throwing errors.

If I replace invalid values with disabled buttons or default settings, it will also generate valid profile. I cannot guarantee that the profile written back is the same as before, either way. For making perfect backup of you profile, use hidpp20-dump-page.

I am not sure I follow... there are three kinds of "0" already... but they are surrounded by or or tags in the XML. I don't see a conflict here. So the QWERTY keyboard "0" would have it within tags.

The ambiguity is only with <key>, you can see the list of accepted strings here. If a key code is missing, it prints the value. 0, 1, 2, 3 can be printed this way because they don't have associated strings. 0 need a special treatment anyway, since it means "no key".