MHeironimus / ArduinoJoystickLibrary

An Arduino library that adds one or more joysticks to the list of HID devices an Arduino Leonardo or Arduino Micro can support.
GNU Lesser General Public License v3.0
2.14k stars 412 forks source link

Using pipe bitwise operator instead of a bunch of bools in constructor #243

Open maduranma opened 2 years ago

maduranma commented 2 years ago

I would suggest using the PIPE (or) bitwise operator to set all the axes you want in the constructor with just one argument, having tons of bools makes it unintuitive.

You could make something like

Joystick_ Joystick(
    JOYSTICK_DEFAULT_REPORT_ID,
    JOYSTICK_TYPE_GAMEPAD,
    8, 0,
    JOYSTICK_INCLUDE_RY_AXIS | JOYSTICK_INCLUDE_RZ_AXIS
);

In the end you end up doing this in:

_includeAxisFlags |= (includeRyAxis ? JOYSTICK_INCLUDE_RY_AXIS : 0);
_includeAxisFlags |= (includeRzAxis ? JOYSTICK_INCLUDE_RZ_AXIS : 0);

It would be cleaner. I can make a PR if you think this is nicer than now. I know you need an Axis count, so I would not use the value, I'd just assign each axis 1, 2, 4, 8, 16, 32... as a constant and then you can generate the value count like this:

#define MAX_AXIS 8

#define JOYSTICK_RY_AXIS 1
#define JOYSTICK_RZ_AXIS 2
#define JOYSTICK_ANOTHER_AXIS 4
// .....

int currentAxis = JOYSTICK_RY_AXIS | JOYSTICK_RZ_AXIS; // this would come by constructor

int axisCount = 0;
for(int i = 0; i < MAX_AXIS; i++)
    if(currentAxis & (int)pow(2, i))
        axisCount++;
MHeironimus commented 2 years ago

I really like this idea. I may apply it to other places as well.