1
\$\begingroup\$

I spent far too much time trying to simply get data from an Xbox controller with Python. This is the cleaned up result, classified. This requires xinput1_3 or possibly 1_4 - I'm running Windows 7 and cannot use 1_4.

import ctypes
import threading
import time
xinput = ctypes.windll.xinput1_3
class controller_manager():
        def __init__(self,checkfrequency=.2,controllernumber = 0,**kwargs):
                """ Xbox 360 controller manager using xinput1_3. kwargs is for functions that should be called on events. If these
        are not defined, it will default to setting these values equal to the current value of that piece."""
                self.state = xinput_state()
                self.controller = controllernumber
                self.variables = ["y_button","x_button","b_button","a_button","right_shoulder","left_shoulder","right_bumper","left_bumper",
                          "right_stick","left_stick","right_dpad","left_dpad","down_dpad","up_dpad","left_trigger","right_trigger",
                          "thumb_lx","thumb_ly","thumb_rx","thumb_ry"]
                for a in kwargs:
                        setattr(self,a,kwargs[a])
                threading.Thread(target=self.control_manager,args=(checkfrequency,)).start()
        def control_manager(self,checkfrequency):
                defined_funcs = {}
                prev_packet = 0
                for a in self.variables:
                        try:
                                defined_funcs[a] = getattr(self,a)
                        except AttributeError:
                                pass
                print(defined_funcs)
                while 1:
                        time.sleep(checkfrequency)
                        xinput.XInputGetState(self.controller,ctypes.pointer(self.state))
                        if self.state.dwPacketNumber == prev_packet:
                                continue
                        prev_packet = self.state.dwPacketNumber
                        res = self.get_state()
                        self.state_dict = res
                        for value in defined_funcs:
                                defined_funcs[value](res[value])
                        for value in [a for a in res if not a in defined_funcs]: # we don't want to override user-defined functions
                                setattr(self,value,res[value])
        def vibrate(self,lmotor,rmotor):
                vibration_struct = xinput_vibration()
                vibration_struct.wLeftMotorSpeed = ctypes.c_ushort(lmotor)
                vibration_struct.wRightMotorSpeed = ctypes.c_ushort(rmotor)
                xinput.XInputSetState(self.controller,vibration_struct)
        def wbuttonstodict(self,wbuttons):
                total = 0x10000
                dicty = {}
                for a in ["y_button","x_button","b_button","a_button","right_shoulder","left_shoulder","right_bumper","left_bumper",
                          "right_stick","left_stick","right_dpad","left_dpad","down_dpad","up_dpad"]:
                        total = total >> 1
                        if wbuttons >= total:
                                wbuttons -= total
                                dicty[a] = True
                        else:
                                dicty[a] = False
                        total >> 1
                return dicty
        def get_state(self):
                m = self.wbuttonstodict(self.state.XINPUT_GAMEPAD.wButtons)
                for a in ["left_trigger","right_trigger","thumb_lx","thumb_ly","thumb_rx","thumb_ry"]:
                        m[a] = getattr(self.state.XINPUT_GAMEPAD,a)
                return m
class xinput_gamepad(ctypes.Structure):
        _fields_ = [("wButtons",ctypes.c_ushort), ("left_trigger",ctypes.c_ubyte), ("right_trigger",ctypes.c_ubyte),("thumb_lx",ctypes.c_short),
                    ("thumb_ly",ctypes.c_short), ("thumb_rx",ctypes.c_short), ("thumb_ry",ctypes.c_short)]
class xinput_state(ctypes.Structure):
    _fields_ = [("dwPacketNumber",ctypes.c_uint),
            ("XINPUT_GAMEPAD",xinput_gamepad)]
class xinput_vibration(ctypes.Structure):
    _fields_ = [("wLeftMotorSpeed",ctypes.c_ushort),
                ("wRightMotorSpeed",ctypes.c_ushort)]
\$\endgroup\$
3
\$\begingroup\$

Readability

This is really hard to read right now. Add some whitespace, trim lines so they are no longer than 79 characters (docstrings no more than 72), etc.

You're over-indenting as well - 8 spaces is far too many.

Scopes

In controller_manager.__init__ you set variables as a list of strings at the instance level - this should probably be at the class level, and you probably want to break them up into separate variables. I did it like this (and you could make it even more generic):

class controller_manager():
    _button_format = "{}_button"
    _shoulder_format = "{}_shoulder"
    _bumper_format = "{}_bumper"
    _stick_format = "{}_stick"
    _dpad_format = "{}_dpad"
    _trigger_format = "{}_trigger"
    _thumb_format = "thumb_{}"

    buttons = ('y', 'x', 'b', 'a')
    shoulders = bumpers = sticks = triggers = ('right', 'left')
    dpads = ('right', 'left', 'down', 'up')
    thumbs = ('lx', 'ly', 'rx', 'ry')

kwargs

You can more cleanly handle the keyword arguments

for name, value in kwargs.items():   # iteritems in Python 2.x, and six.iteritems(kwargs) for compatibility
    setattr(self, name, value)

Repetition

You repeat very similar lines of code as well, and they can be simplified, for example into

attribute = getattr(self, variable_name)
if attribute is not None:
    defined_funcs[variable_name] = attribute

or

def wbuttonstodict(self, wbuttons):
    total = 0x10000
    enabled_options = {}
    variable_options = [
        option for option in self.variables if option not in self.thumbs
    ]
    for option in variable_options:
        total >>= 1
        condition = wbuttons >= total
        enabled_options[1] = condition
        wbuttons -= total if condition else 0
        total >> 1

    return enabled_options

etc.

I don't know enough about the domain to comment towards correctness or if there is an easier way to handle these things, but in terms of readability and general Pythonicness, here is a full rewrite of things I would change.

import ctypes
import threading
import time


xinput = ctypes.windll.xinput1_3


class ControllerManager():
    buttons = map("{}_button".format, ('y', 'x', 'b', 'a'))
    shoulders = map("{}_shoulder".format, ('right', 'left'))
    dpads = map("{}_bumper".format, ('right', 'left'))
    sticks = map("{}_stick".format, ('right', 'left'))
    bumpers = map("{}_bumper".format, ('right', 'left'))
    triggers = map("{}_trigger".format, ('right', 'left'))
    thumbs = map("thumb_{}".format, ('lx', 'ly', 'rx', 'ry'))

    variables = buttons + shoulders + dpads + sticks + bumpers + triggers + thumbs

    def __init__(self, checkfrequency=.2, controllernumber=0, **kwargs):
        """Xbox 360 controller manager using xinput1_3. kwargs
        is for functions that should be called on events. If 
        these are not defined, it will default to setting these
        values equal to the current value of that piece."""

        self.state = xinput_state()
        self.controller = controllernumber

        for name, value in kwargs.iteritems():
            setattr(self, name, value)

        threading.Thread(
            target=self.control_manager, args=(checkfrequency,)
        ).start()

    def control_manager(self, checkfrequency):
        defined_functions = {}
        prev_packet = 0
        for variable_name in self.variables:
            attribute = getattr(self, variable_name)
            if attribute is not None:
                defined_funcs[variable_name] = attribute

        while True:
            time.sleep(checkfrequency)
            xinput.XInputGetState(self.controller, ctypes.pointer(self.state))

            if self.state.dwPacketNumber == prev_packet:
                continue

            prev_packet = self.state.dwPacketNumber
            self.state_dict = self.get_state()

            for function_name, function in defined_funcs.iteritems():
                function(self.state_dict[function_name])

            selected_results = (result 
                                for result in self.state_dict 
                                if not result in defined_funcs)

            for value in selected_results:
                setattr(self, value, self.state_dict[value])

    def vibrate(self, lmotor, rmotor):
        vibration_struct = xinput_vibration()
        vibration_struct.wLeftMotorSpeed = ctypes.c_ushort(lmotor)
        vibration_struct.wRightMotorSpeed = ctypes.c_ushort(rmotor)
        xinput.XInputSetState(self.controller,vibration_struct)

    def wbuttonstodict(self, wbuttons):
        total = 0x10000
        enabled_options = {}
        variable_options = [
            option for option in self.variables if option not in self.thumbs
        ]
        for option in variable_options:
            total >>= 1
            condition = wbuttons >= total
            enabled_options[1] = condition
            wbuttons -= total if condition else 0
            total >> 1

        return enabled_options

    def get_state(self):
        m = self.wbuttonstodict(self.state.XINPUT_GAMEPAD.wButtons)
        for option_name in (self.triggers + self.thumbs):
            m[option_name] = getattr(self.state.XINPUT_GAMEPAD, option_name)
        return m


class xinput_gamepad(ctypes.Structure):
    _fields_ = [("wButtons", ctypes.c_ushort), ("left_trigger", ctypes.c_ubyte), 
                ("right_trigger", ctypes.c_ubyte), ("thumb_lx", ctypes.c_short),
                ("thumb_ly", ctypes.c_short), ("thumb_rx", ctypes.c_short), 
                ("thumb_ry", ctypes.c_short)]


class xinput_state(ctypes.Structure):
    _fields_ = [("dwPacketNumber", ctypes.c_uint),
                 ("XINPUT_GAMEPAD", xinput_gamepad)]


class xinput_vibration(ctypes.Structure):
    _fields_ = [("wLeftMotorSpeed", ctypes.c_ushort),
                ("wRightMotorSpeed", ctypes.c_ushort)]
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.