Merge lp:~jeff-licquia/pystromo/saitek into lp:pystromo

Proposed by Jeff Licquia
Status: Needs review
Proposed branch: lp:~jeff-licquia/pystromo/saitek
Merge into: lp:pystromo
Diff against target: 442 lines (+325/-8)
4 files modified
config/saitek-cyborg-cmdunit.map (+236/-0)
lib/constants.py (+15/-0)
lib/devices.py (+70/-7)
pystromo-remap.py (+4/-1)
To merge this branch: bzr merge lp:~jeff-licquia/pystromo/saitek
Reviewer Review Type Date Requested Status
Raumkraut Needs Fixing
Review via email: mp+41413@code.launchpad.net

Description of the change

This branch pulls in the changes from Shane McCormack to support the Saitek Cyborg Command Unit gamepad (with some updates from me).

To post a comment you must log in.
Revision history for this message
Raumkraut (raumkraut) wrote :

Thanks for bringing this to my attention, Jeff.
It's not going to get merged in though, as the features have been implemented in the wrong places:
- Parsing config files should be (and is!) done before an InputDevice is instantiated.
- Saving application-state information shouldn't really be done by the InputDevice either. And it's probably a bad idea to write config data every time an LED changes.
- Plus some nomenclature issues. :)

And I have to ask: why were some buttons renamed to "TRIGGER_HAPPY##", aside from the amusement factor? :)

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

On 11/23/2010 07:48 AM, Raumkraut wrote:
> Review: Needs Information
> Thanks for bringing this to my attention, Jeff.
> It's not going to get merged in though, as the features have been implemented in the wrong places:
> - Parsing config files should be (and is!) done before an InputDevice is instantiated.
> - Saving application-state information shouldn't really be done by the InputDevice either. And it's probably a bad idea to write config data every time an LED changes.
> - Plus some nomenclature issues. :)

OK. I'll refactor and resend. Any hints on the nomenclature issues?

> And I have to ask: why were some buttons renamed to "TRIGGER_HAPPY##", aside from the amusement factor? :)

It's from linux/input.h; here's the commit log for when this came in:

commit cf2f765f1896064e34c6f0f2ef896ff058dd5c06
Author: Jiri Kosina <email address hidden>
Date: Mon Jan 4 12:20:56 2010 +0100

     HID: handle joysticks with large number of buttons

     Current HID code doesn't properly handle HID joysticks which have
     larger number of buttons than what fits into current range reserved
     for BTN_JOYSTICK.

     One such joystick reported to not work properly is Saitek X52 Pro
     Flight System.

     We can't extend the range to fit more buttons in, because of backwards
     compatibility reasons.

     Therefore this patch introduces a new BTN_TRIGGER_HAPPY range, and
     uses these to map the buttons which are over BTN_JOYSTICK limit.

     Acked-by: Dmitry Torokhov <email address hidden> [for the
input.h part]
     Signed-off-by: Jiri Kosina <email address hidden>

Revision history for this message
Raumkraut (raumkraut) wrote :

Right, further clarification:
- 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)
- 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.
- Nomenclature:
- - saitek.map should be named more specifically for the device.
- - 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.
- - Plus any other variables named after device manufacturers. ;)
- - And if the Czech wants to go all TRIGGER_HAPPY, I guess we should let him. :)

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

Phew! It probably took me more time to write that, than it will for you to refactor the code. :D

review: Needs Fixing
lp:~jeff-licquia/pystromo/saitek updated
66. By Jeff Licquia

First pass at refactoring according to suggestions from upstream.

Changes:

 - References to "saitek" in variable names purged.

 - Instead of configuring the saved state file, force it to a standard
   location (currently ~/.local/pystromo, set in constants.py), with a
   standard filename.

 - Instead of saving on every LED state change, save when pystromo-remap
   exits. State is also only saved if needed (i.e. we read state on startup
   and/or we detected a state change).

67. By Jeff Licquia

Include further changes in key mapping for LED "buttons".

68. By Jeff Licquia

Fix bug in InputDevice.save_state(), and remove debug statements.

69. By Jeff Licquia

Renamed sample file and gave it a better device ID.

70. By Jeff Licquia

Get rid of print statements in lib dir.

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.

Unmerged revisions

70. By Jeff Licquia

Get rid of print statements in lib dir.

69. By Jeff Licquia

Renamed sample file and gave it a better device ID.

68. By Jeff Licquia

Fix bug in InputDevice.save_state(), and remove debug statements.

67. By Jeff Licquia

Include further changes in key mapping for LED "buttons".

66. By Jeff Licquia

First pass at refactoring according to suggestions from upstream.

Changes:

 - References to "saitek" in variable names purged.

 - Instead of configuring the saved state file, force it to a standard
   location (currently ~/.local/pystromo, set in constants.py), with a
   standard filename.

 - Instead of saving on every LED state change, save when pystromo-remap
   exits. State is also only saved if needed (i.e. we read state on startup
   and/or we detected a state change).

65. By Jeff Licquia

Updated key constants for buttons F1-F4 and 21 on Saitek.

64. By Shane Mc Cormack

Pull in Saitek gamepad changes from Shane McCormack.

Original repo: git://git.soren.co.uk/shane/pystromo.git/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== added file 'config/saitek-cyborg-cmdunit.map'
2--- config/saitek-cyborg-cmdunit.map 1970-01-01 00:00:00 +0000
3+++ config/saitek-cyborg-cmdunit.map 2011-01-30 05:34:10 +0000
4@@ -0,0 +1,236 @@
5+# Saitek Cyborg Command Unit gamepad
6+[Device:saitek-cyborg-command-unit]
7+vendor=0x06a3
8+product=0x80c1
9+leds=BTN_TRIGGER_HAPPY22, BTN_TRIGGER_HAPPY23, BTN_TRIGGER_HAPPY24
10+
11+################################################################################
12+# Green #
13+################################################################################
14+[map:saitek-cyborg-command-unit + BTN_TRIGGER_HAPPY22]
15+### F1
16+#BTN_TRIGGER_HAPPY17:
17+### F2
18+#BTN_TRIGGER_HAPPY18:
19+### F3
20+#BTN_TRIGGER_HAPPY19:
21+### F4
22+#BTN_TRIGGER_HAPPY20:
23+
24+### 1
25+#BTN_TRIGGER:
26+### W
27+#BTN_THUMB:
28+### 3
29+#BTN_THUMB2:
30+### 4
31+#BTN_TOP:
32+
33+### A
34+#BTN_TOP2:
35+### S
36+#BTN_PINKIE:
37+### D
38+#BTN_BASE:
39+### 8
40+#BTN_BASE2:
41+
42+### 9
43+#BTN_BASE3:
44+### 10
45+#BTN_BASE4:
46+### 11
47+#BTN_BASE5:
48+
49+### 12
50+#BTN_BASE6:
51+### 13
52+#BTN_7:
53+### 14
54+#BTN_8:
55+
56+### 15
57+#BTN_DEAD2:
58+### 16
59+#BTN_DEAD:
60+### 21
61+#BTN_TRIGGER_HAPPY21:
62+
63+### X Axis
64+#ABS_X:
65+### Y Axis
66+#ABS_Y:
67+
68+
69+################################################################################
70+# Yellow #
71+################################################################################
72+[map:saitek-cyborg-command-unit + BTN_TRIGGER_HAPPY23]
73+### F1
74+#BTN_TRIGGER_HAPPY17:
75+### F2
76+#BTN_TRIGGER_HAPPY18:
77+### F3
78+#BTN_TRIGGER_HAPPY19:
79+### F4
80+#BTN_TRIGGER_HAPPY20:
81+
82+### 1
83+#BTN_TRIGGER:
84+### W
85+#BTN_THUMB:
86+### 3
87+#BTN_THUMB2:
88+### 4
89+#BTN_TOP:
90+
91+### A
92+#BTN_TOP2:
93+### S
94+#BTN_PINKIE:
95+### D
96+#BTN_BASE:
97+### 8
98+#BTN_BASE2:
99+
100+### 9
101+#BTN_BASE3:
102+### 10
103+#BTN_BASE4:
104+### 11
105+#BTN_BASE5:
106+
107+### 12
108+#BTN_BASE6:
109+### 13
110+#BTN_7:
111+### 14
112+#BTN_8:
113+
114+### 15
115+#BTN_DEAD2:
116+### 16
117+#BTN_DEAD:
118+### 21
119+#BTN_TRIGGER_HAPPY21:
120+
121+### X Axis
122+#ABS_X:
123+### Y Axis
124+#ABS_Y:
125+
126+
127+################################################################################
128+# Red #
129+################################################################################
130+[map:saitek-cyborg-command-unit + BTN_TRIGGER_HAPPY24]
131+### F1
132+#BTN_TRIGGER_HAPPY17:
133+### F2
134+#BTN_TRIGGER_HAPPY18:
135+### F3
136+#BTN_TRIGGER_HAPPY19:
137+### F4
138+#BTN_TRIGGER_HAPPY20:
139+
140+### 1
141+#BTN_TRIGGER:
142+### W
143+#BTN_THUMB:
144+### 3
145+#BTN_THUMB2:
146+### 4
147+#BTN_TOP:
148+
149+### A
150+#BTN_TOP2:
151+### S
152+#BTN_PINKIE:
153+### D
154+#BTN_BASE:
155+### 8
156+#BTN_BASE2:
157+
158+### 9
159+#BTN_BASE3:
160+### 10
161+#BTN_BASE4:
162+### 11
163+#BTN_BASE5:
164+
165+### 12
166+#BTN_BASE6:
167+### 13
168+#BTN_7:
169+### 14
170+#BTN_8:
171+
172+### 15
173+#BTN_DEAD2:
174+### 16
175+#BTN_DEAD:
176+### 21
177+#BTN_TRIGGER_HAPPY21:
178+
179+### X Axis
180+#ABS_X:
181+### Y Axis
182+#ABS_Y:
183+
184+
185+################################################################################
186+# All #
187+################################################################################
188+[map:saitek-cyborg-command-unit]
189+### F1
190+#BTN_TRIGGER_HAPPY17:
191+### F2
192+#BTN_TRIGGER_HAPPY18:
193+### F3
194+#BTN_TRIGGER_HAPPY19:
195+### F4
196+#BTN_TRIGGER_HAPPY20:
197+
198+### 1
199+#BTN_TRIGGER:
200+### W
201+#BTN_THUMB:
202+### 3
203+#BTN_THUMB2:
204+### 4
205+#BTN_TOP:
206+
207+### A
208+#BTN_TOP2:
209+### S
210+#BTN_PINKIE:
211+### D
212+#BTN_BASE:
213+### 8
214+#BTN_BASE2:
215+
216+### 9
217+#BTN_BASE3:
218+### 10
219+#BTN_BASE4:
220+### 11
221+#BTN_BASE5:
222+
223+### 12
224+#BTN_BASE6:
225+### 13
226+#BTN_7:
227+### 14
228+#BTN_8:
229+
230+### 15
231+#BTN_DEAD2:
232+### 16
233+#BTN_DEAD:
234+### 21
235+#BTN_TRIGGER_HAPPY21:
236+
237+### X Axis
238+#ABS_X:
239+### Y Axis
240+#ABS_Y:
241
242=== modified file 'lib/constants.py'
243--- lib/constants.py 2007-10-01 13:58:18 +0000
244+++ lib/constants.py 2011-01-30 05:34:10 +0000
245@@ -15,6 +15,10 @@
246 # The first one which exists is the one which is used.
247 UINPUT_DEVICES = ['/dev/uinput', '/dev/input/uinput', '/dev/misc/uinput']
248
249+# Path to the device state file, if used. This is relative to the home
250+# directory.
251+STATE_FILE_PATH = "~/.local/pystromo"
252+
253
254
255 ## I don't know any other bus values!
256@@ -299,6 +303,9 @@
257 0x129: 'BTN_BASE4',
258 0x12a: 'BTN_BASE5',
259 0x12b: 'BTN_BASE6',
260+ 0x12c: 'BTN_BASE7',
261+ 0x12d: 'BTN_BASE8',
262+ 0x12e: 'BTN_DEAD2',
263 0x12f: 'BTN_DEAD',
264 0x130: 'BTN_A',
265 0x131: 'BTN_B',
266@@ -326,6 +333,14 @@
267 0x14a: 'BTN_TOUCH',
268 0x14b: 'BTN_STYLUS',
269 0x14c: 'BTN_STYLUS2',
270+ 0x2d0: 'BTN_TRIGGER_HAPPY17',
271+ 0x2d1: 'BTN_TRIGGER_HAPPY18',
272+ 0x2d2: 'BTN_TRIGGER_HAPPY19',
273+ 0x2d3: 'BTN_TRIGGER_HAPPY20',
274+ 0x2d4: 'BTN_TRIGGER_HAPPY21',
275+ 0x2d5: 'BTN_TRIGGER_HAPPY22',
276+ 0x2d6: 'BTN_TRIGGER_HAPPY23',
277+ 0x2d7: 'BTN_TRIGGER_HAPPY24',
278 },
279 # EV_REL
280 0x02: {
281
282=== modified file 'lib/devices.py'
283--- lib/devices.py 2008-10-13 22:00:38 +0000
284+++ lib/devices.py 2011-01-30 05:34:10 +0000
285@@ -9,6 +9,8 @@
286 import events
287 import mapping
288 import constants as const
289+import ConfigParser
290+import os
291
292 # This is a list of all the devices which have been created.
293 # We use this for the receive() function.
294@@ -59,7 +61,7 @@
295 busy = property(fget=_getBusy)
296
297
298- def __init__ (self, keymap, id=None, output=None, **params):
299+ def __init__ (self, keymap, id=None, output=None, leds=None, **params):
300 """
301 If not specified, the id will be populated by the device's
302 detected 'name' parameter (if any).
303@@ -90,9 +92,18 @@
304 # at the same time...
305 self._starting = set()
306
307+ # Some gamepads use button events to indicate the state of
308+ # a switch on the pad. Instead of recording individual
309+ # events, we should maintain an internal sense of the
310+ # switch state which can be used in key definitions.
311+ if leds:
312+ self._switch_buttons = leds.replace(' ', '').split(',');
313+ else:
314+ self._switch_buttons = []
315
316 # Do stuff with device nodes
317- nodes = ioctl.matchInputNodes (**params)
318+ # nodes = ioctl.matchInputNodes (**params)
319+ nodes = ioctl.matchInputNodes (vendor=params['vendor'], product=params['product'])
320 self._nodes = nodes
321 for node in nodes:
322 # Gimme gimme gimme!
323@@ -129,7 +140,29 @@
324 # Shortcuts
325 self._remap = lambda chord, **kwargs: self.keymap.remap(chord, device=self.id, **kwargs)
326 self._inMap = lambda chord: self.keymap.deviceContains(chord, device=self.id)
327-
328+
329+ # State file?
330+ self._led_state = None
331+ self._state_fn = None
332+ if self.id:
333+ self._state_fn = self.id + ".state"
334+ elif "vendor" in params and "product" in params:
335+ self._state_fn = "%s:%s.state" % (params["vendor"], params["product"])
336+
337+ if self._state_fn:
338+ state_path = os.path.expanduser(os.path.join(const.STATE_FILE_PATH, self._state_fn))
339+ if os.path.exists(state_path):
340+ savefile = ConfigParser.ConfigParser()
341+ savefile.read(state_path)
342+ if savefile.has_section('lastled'):
343+ try:
344+ stringCode = savefile.get('lastled', 'stringCode')
345+ eventValue = savefile.get('lastled', 'eventValue')
346+ self._led_state = (stringCode, eventValue)
347+ key = mapping.Key(stringCode, eventValue, eventValue)
348+ self._changeMode(key)
349+ except KeyError:
350+ pass
351
352 def __repr__ (self):
353 params = (self.__class__.__name__, str(self.id), len(self._nodes))
354@@ -141,6 +174,28 @@
355 Returns a list of all the device nodes associated with this object.
356 """
357 return self._nodes
358+
359+
360+ def save_state(self):
361+ """
362+ Saves LED state for the device, if supported.
363+ """
364+ if self._state_fn and self._led_state:
365+ (stringCode, eventValue) = self._led_state
366+ savefile = ConfigParser.ConfigParser()
367+
368+ state_dir = os.path.expanduser(const.STATE_FILE_PATH)
369+ if not os.path.exists(state_dir):
370+ os.makedirs(state_dir)
371+
372+ state_path = os.path.join(state_dir, self._state_fn)
373+ if os.path.exists(state_path):
374+ savefile.read(state_path)
375+ if not savefile.has_section('lastled'):
376+ savefile.add_section('lastled')
377+ savefile.set('lastled', 'stringCode', stringCode)
378+ savefile.set('lastled', 'eventValue', eventValue)
379+ savefile.write(open(state_path, 'wb'));
380
381
382 def read (self, node=None):
383@@ -188,7 +243,7 @@
384 events.append(event)
385
386 bytesToRead = eventSize
387- data = ''
388+ data = ""
389
390
391 return events
392@@ -208,6 +263,12 @@
393 ## Most likely a SYN packet. We see a lot of those.
394 return False
395
396+ # Some game pads send BTN events for a change in LED mode
397+ # instead of EV_LED events, handle that nicely here if the
398+ # appropriate config settings are set.
399+ if (event.type == const.EV_KEY and stringCode in self._switch_buttons):
400+ event.type = const.EV_LED
401+
402 if event.type == const.EV_KEY and self._inMap(key):
403 if event.value == const.KEYDOWN:
404 self._startEvent (key)
405@@ -239,8 +300,11 @@
406
407
408 elif event.type == const.EV_LED:
409- # We've received a notification of a change in LED status
410- self._changeMode (key)
411+ # Change in LED status
412+ if event.value == const.KEYDOWN:
413+ self._led_state = (stringCode, event.value)
414+ self._changeMode(key)
415+
416
417 else:
418 # Don't have any specific rules for this type
419@@ -258,7 +322,6 @@
420 if cycle not in queue:
421 queue[cycle] = cycle.getIter()
422
423-
424 def _startEvent (self, key, modes=None):
425 """
426 Starts whatever needs to be started with the additional
427
428=== modified file 'pystromo-remap.py'
429--- pystromo-remap.py 2008-10-13 22:00:38 +0000
430+++ pystromo-remap.py 2011-01-30 05:34:10 +0000
431@@ -211,7 +211,10 @@
432 # End the loop only if all the devices have finished.
433 if not len(busyDevices):
434 looping = False
435-
436+
437+# Save our state, if needed
438+for device in inputs:
439+ device.save_state()
440
441 # Destroy and close our output
442 output.close()

Subscribers

People subscribed via source and target branches

to all changes: