Code review comment for lp:~jeff-licquia/pystromo/saitek

Revision history for this message
Jeff Licquia (jeff-licquia) wrote :

Hello again! Sorry for taking so long, but here's the refactored changes. I tried to do everything you asked for. Specifics:

> - The events to use as additional LEDs should be passed in to InputDevices on
> instantiation. Actually, it looks like the value is already implicitly passed
> in with the **params. (The **params were originally intended solely to
> identify the devices to match against, so 'leds' may be better as another
> argument, I'm not sure)

"leds" is now its own argument.

> - The LED state should probably only be saved on exit, which I think just
> means moving that code to the end of pystromo-remap.py.

Done, sorta. The save code was moved to its own method for InputDevice, which is called at the end of pystromo-remap.py.

> - Nomenclature:
> - - saitek.map should be named more specifically for the device.

Done.

> - - The location of the save-file should probably not be user-specified, but
> in constants.py. Following XDG probably means ~/.local/pystromo/{device-
> id}.state (because we're storing the "state" of the device). If there's no
> device-id available (I forget), "{vendor}:{product}.state" probably covers
> 99.99% of cases.

This is done exactly as requested.

> - - Plus any other variables named after device manufacturers. ;)

Think I got them all.

> - - And if the Czech wants to go all TRIGGER_HAPPY, I guess we should let him.
> :)

It turned out I needed to add a few more; the switch events moved along with some of the buttons.

> Also: `print` statements should probably be removed/commented from anything in
> the 'lib' directory, once you're happy that it's working.

Think I got them all.

« Back to merge proposal