search

pablomarquez76 / PS4_Controller_Host

Allows ESP32 to communicate with PS4 controller (can be used to control robots and other devices)
GNU General Public License v3.0
30 stars 5 forks source link

PS4Controller::setFlashRate has an inappropriate argument type, which limits the settable time. #2

Open KOJ-Factory opened 11 months ago

KOJ-Factory commented 11 months ago

It says that you can specify a maximum of 2550ms, but because it is uint8_t, can only specify up to 0.255 seconds. It is better to use uint16_t.

pablomarquez76 commented 11 months ago

You are right. That is a bug in the code, but since the data accepted by the controller is a uint8_t , I fixed the code to match the controller's (from release 1.0.8 onwards) , now 255 = 2550ms .

The data structure for state data is the following:

struct BTSetStateData { uint8_t EnableRumbleUpdate : 1; uint8_t EnableLedUpdate : 1; uint8_t EnableLedBlink : 1; uint8_t EnableExtWrite : 1; uint8_t EnableVolumeLeftUpdate : 1; uint8_t EnableVolumeRightUpdate : 1; uint8_t EnableVolumeMicUpdate : 1; uint8_t EnableVolumeSpeakerUpdate : 1; uint8_t UNK_RESET1: 1; // unknown reset, both set high by Remote Play uint8_t UNK_RESET2: 1; // unknown reset, both set high by Remote Play uint8_t UNK1: 1; uint8_t UNK2: 1; uint8_t UNK3: 1; uint8_t UNKPad: 3; uint8_t Empty1; uint8_t RumbleRight; // weak uint8_t RumbleLeft; // strong uint8_t LedRed; uint8_t LedGreen uint8_t LedBlue; uint8_t LedFlashOnPeriod; uint8_t LedFlashOffPeriod; uint8_t ExtDataSend[8]; // sent to I2C EXT port, stored in 8x8 byte block uint8_t VolumeLeft; // 0x00 - 0x4F inclusive uint8_t VolumeRight; // 0x00 - 0x4F inclusive uint8_t VolumeMic; // 0x00, 0x01 - 0x40 inclusive (0x00 is special behavior) uint8_t VolumeSpeaker; // 0x00 - 0x4F uint8_t UNK_AUDIO1 : 7; // clamped to 1-64 inclusive, appears to be set to 5 for audio uint8_t UNK_AUDIO2: 1; // unknown, appears to be set to 1 for audio uint8_t Pad[52]; };

KOJ-Factory commented 11 months ago

Thanks for the correction. However, this will cause incompatibility with Ver1.0.7, so I think it would be better to receive the argument as uint16_t and pass it to the controller as uint8_t, which would have less impact.

pablomarquez76 commented 11 months ago

Thanks for you comments! I don't think it would be a problem since it wasn't working the way it should in previous versions. So, to make the code more efficient in terms of memory usage and computing effort it may be better to leave it as uint8_t .