From 65f6ee18cab1c4bff7a6bf9695c93181856c2749 Mon Sep 17 00:00:00 2001 From: afpineda <74291754+afpineda@users.noreply.github.com> Date: Thu, 19 Dec 2024 18:30:34 +0100 Subject: [PATCH] Fix NimBLEHIDDevice not being able to create more than one in/out/feature report + temporal coupling --- .gitignore | 2 +- src/NimBLEHIDDevice.cpp | 56 +++++++++++++++++++++++++++++++++-------- src/NimBLEHIDDevice.h | 2 ++ 3 files changed, 49 insertions(+), 11 deletions(-) diff --git a/.gitignore b/.gitignore index 343d1f4..111405a 100644 --- a/.gitignore +++ b/.gitignore @@ -1 +1 @@ -docs/doxydocs \ No newline at end of file +docs/doxydocs diff --git a/src/NimBLEHIDDevice.cpp b/src/NimBLEHIDDevice.cpp index a6416f1..e07efc4 100644 --- a/src/NimBLEHIDDevice.cpp +++ b/src/NimBLEHIDDevice.cpp @@ -149,14 +149,41 @@ void NimBLEHIDDevice::setBatteryLevel(uint8_t level, bool notify) { } } // setBatteryLevel +/** + * @brief Locate the characteristic for a report ID. + * + * @param [in] reportId Report identifier to locate. + * @param [out] reportType Type of report (input/output/feature). Not meaningful if the return value is nullptr. + * @return NimBLECharacteristic* The characteristic. + * @return nullptr If the characteristic does not exist. + */ +NimBLECharacteristic* NimBLEHIDDevice::locateReportCharacteristicById(uint8_t reportId, uint8_t& reportType) { + NimBLECharacteristic* candidate = m_hidSvc->getCharacteristic(inputReportChrUuid, 0); + for (uint16_t i = 1; (candidate != nullptr) && (i != 0); i++) { + NimBLEDescriptor* dsc = candidate->getDescriptorByUUID(featureReportDscUuid); + NimBLEAttValue desc1_val_att = dsc->getValue(); + const uint8_t* desc1_val = desc1_val_att.data(); + reportType = desc1_val[1]; + if (desc1_val[0] == reportId) return candidate; + candidate = m_hidSvc->getCharacteristic(inputReportChrUuid, i); + } + return nullptr; +} + /** * @brief Get the input report characteristic. - * @param [in] reportId input report ID, the same as in report map for input object related to the characteristic. - * @return A pointer to the input report characteristic. + * @param [in] reportId Input report ID, the same as in report map for input object related to the characteristic. + * @return NimBLECharacteristic* A pointer to the input report characteristic. + * Store this value to avoid computational overhead. + * @return nullptr If the report is already created as an output or feature report. * @details This will create the characteristic if not already created. */ NimBLECharacteristic* NimBLEHIDDevice::getInputReport(uint8_t reportId) { - NimBLECharacteristic* inputReportChr = m_hidSvc->getCharacteristic(inputReportChrUuid); + uint8_t reportType; + NimBLECharacteristic* inputReportChr = locateReportCharacteristicById(reportId, reportType); + if ((inputReportChr != nullptr) && (reportType != 0x01)) + // ERROR: this reportId exists, but it is not an input report + return nullptr; if (inputReportChr == nullptr) { inputReportChr = m_hidSvc->createCharacteristic(inputReportChrUuid, @@ -174,13 +201,17 @@ NimBLECharacteristic* NimBLEHIDDevice::getInputReport(uint8_t reportId) { /** * @brief Get the output report characteristic. * @param [in] reportId Output report ID, the same as in report map for output object related to the characteristic. - * @return A pointer to the output report characteristic. + * @return NimBLECharacteristic* A pointer to the output report characteristic. + * Store this value to avoid computational overhead. + * @return nullptr If the report is already created as an input or feature report. * @details This will create the characteristic if not already created. - * @note The output report characteristic is optional and should only be created after the input report characteristic. */ NimBLECharacteristic* NimBLEHIDDevice::getOutputReport(uint8_t reportId) { - // Same uuid as input so this needs to be the second instance - NimBLECharacteristic* outputReportChr = m_hidSvc->getCharacteristic(inputReportChrUuid, 1); + uint8_t reportType; + NimBLECharacteristic* outputReportChr = locateReportCharacteristicById(reportId, reportType); + if ((outputReportChr != nullptr) && (reportType != 0x02)) + // ERROR: this reportId exists, but it is not an output report + return nullptr; if (outputReportChr == nullptr) { outputReportChr = m_hidSvc->createCharacteristic(inputReportChrUuid, @@ -189,7 +220,6 @@ NimBLECharacteristic* NimBLEHIDDevice::getOutputReport(uint8_t reportId) { NimBLEDescriptor* outputReportDsc = outputReportChr->createDescriptor( featureReportDscUuid, NIMBLE_PROPERTY::READ | NIMBLE_PROPERTY::WRITE | NIMBLE_PROPERTY::READ_ENC | NIMBLE_PROPERTY::WRITE_ENC); - uint8_t desc1_val[] = {reportId, 0x02}; outputReportDsc->setValue(desc1_val, 2); } @@ -200,11 +230,17 @@ NimBLECharacteristic* NimBLEHIDDevice::getOutputReport(uint8_t reportId) { /** * @brief Get the feature report characteristic. * @param [in] reportId Feature report ID, the same as in report map for feature object related to the characteristic. - * @return A pointer to feature report characteristic. + * @return NimBLECharacteristic* A pointer to feature report characteristic. + * Store this value to avoid computational overhead. + * @return nullptr If the report is already created as an input or output report. * @details This will create the characteristic if not already created. */ NimBLECharacteristic* NimBLEHIDDevice::getFeatureReport(uint8_t reportId) { - NimBLECharacteristic* featureReportChr = m_hidSvc->getCharacteristic(inputReportChrUuid); + uint8_t reportType; + NimBLECharacteristic* featureReportChr = locateReportCharacteristicById(reportId, reportType); + if ((featureReportChr != nullptr) && (reportType != 0x03)) + // ERROR: this reportId exists, but it is not a feature report + return nullptr; if (featureReportChr == nullptr) { featureReportChr = m_hidSvc->createCharacteristic( inputReportChrUuid, diff --git a/src/NimBLEHIDDevice.h b/src/NimBLEHIDDevice.h index 0737e75..9b97cd7 100644 --- a/src/NimBLEHIDDevice.h +++ b/src/NimBLEHIDDevice.h @@ -81,6 +81,8 @@ class NimBLEHIDDevice { NimBLECharacteristic* m_hidControlChr{nullptr}; // 0x2a4c NimBLECharacteristic* m_protocolModeChr{nullptr}; // 0x2a4e NimBLECharacteristic* m_batteryLevelChr{nullptr}; // 0x2a19 + + NimBLECharacteristic* locateReportCharacteristicById(uint8_t reportId, uint8_t& reportType); }; #endif // CONFIG_BT_ENABLED && CONFIG_BT_NIMBLE_ROLE_BROADCASTER && defined(CONFIG_BT_NIMBLE_ROLE_PERIPHERAL)