YukMingLaw / ArduinoJoystickWithFFBLibrary

An Arduino Joystick Library With Force Feedback Feature
GNU Lesser General Public License v3.0
208 stars 55 forks source link

Set Periodic period size mismatch #31

Closed sgtnoodle closed 2 years ago

sgtnoodle commented 3 years ago

The descriptor for period is specified as 32 bits:

    0x09, 0x72,           //  Usage (Period)
    0x15, 0x00,           //   Logical Minimum (0)
    0x27, 0xFF, 0x7F, 0, 0, //   Logical Maximum (32K)
    0x35, 0x00,           //   Physical Minimum (0)
    0x47, 0xFF, 0x7F, 0, 0, //   Physical Maximum (32K)
    0x66, 0x03, 0x10,     //   Unit (1003h) (English Linear, Seconds)
    0x55, 0xFD,           //   Unit Exponent (FDh) (X10^-3 ==> Milisecond)
    0x75, 0x20,           //   Report Size (32)
    0x95, 0x01,           //   Report Count (1)
    0x91, 0x02,           //   Output (Data,Var,Abs)
    0x66, 0x00, 0x00,     //  Unit (0)
    0x55, 0x00,           //  Unit Exponent (0)

But the corresponding struct uses a uint16_t:

typedef struct//FFB: Set Periodic Output Report
{
    uint8_t reportId;   // =4
    uint8_t effectBlockIndex;   // 1..40
    uint16_t magnitude;
    int16_t offset;
    uint16_t    phase;  // 0..255 (=0..359, exp-2)
    uint16_t    period; // 0..32767 ms
} USB_FFBReport_SetPeriodic_Output_Data_t;

This seems to happen to work because the data is little endian, the range is limited to 0-32767, and the code is simply reinterpret casting the raw data to the struct. It seems like it's probably worth either fixing the descriptor, using a uint32_t in the struct for consistency, or at least adding a comment to the struct, because otherwise it's alarming to stumble upon it when debugging.

YukMingLaw commented 2 years ago

Thank you for your reminder!It fixs now.