Sensirion / arduino-ble-gadget

Create your own Do-It-Yourself BLE enabled sensor gadget on the ESP32 platform.
BSD 3-Clause "New" or "Revised" License
63 stars 20 forks source link

Issue with gaps downloading data with latest library version and MyAmbiance 3.3.0 (86) #36

Closed melkati closed 5 months ago

melkati commented 5 months ago

Hello,

I'm encountering an issue with the latest version of the library. When the ESP32 device sends data to my Android phone using MyAmbiance 3.3.0 (86), I'm observing data gaps, with one gap appearing for every correct data point.

Image

After extensive testing, it seems that the feature works fine when using the library version before your last "big change."

I found it is working fine using in platformio's lib_deps with these versions:

https://github.com/Sensirion/arduino-upt-core.git#b2c0e76  ; Fix for arduino-ble-gadget incompatibly with arduino-upt-core library
https://github.com/Sensirion/arduino-ble-gadget.git#eb9d348

I've been debugging this for about a week now and haven't been able to pinpoint the exact cause of the issue. However, I suspect it may be related to the library.

To compile with the old arduino-upt-core library (as it was before the change from converterFunctionto encodingFunction), I made the following necessary changes:

void DataProvider::writeValueToCurrentSample(float value,
                                             SignalType signalType) {
    // Check for valid value
    if (isnan(value)) {
        return;
    }

    // Check for correct signal type
    if (_sampleConfig.sampleSlots.count(signalType) ==
        0) { // implies signal type is not part of sample
        return;
    }

    // Get relevant metaData
    uint16_t (*encodingFunction)(float value) =
        _sampleConfig.sampleSlots.at(signalType).encodingFunction;
    size_t offset = _sampleConfig.sampleSlots.at(signalType).offset;

    // Convert float to 16 bit int
    uint16_t convertedValue = encodingFunction(value);
    _currentSample.writeValue(convertedValue, offset);
}

into:

void DataProvider::writeValueToCurrentSample(float value,
                                             SignalType signalType) {
    // Check for valid value
    if (isnan(value)) {
        return;
    }

    // Check for correct signal type
    if (_sampleConfig.sampleSlots.count(signalType) ==
        0) { // implies signal type is not part of sample
        return;
    }

    // Get relevant metaData
    uint16_t (*converterFunction)(float value) =
        _sampleConfig.sampleSlots.at(signalType).converterFunction;
    size_t offset = _sampleConfig.sampleSlots.at(signalType).offset;

    // Convert float to 16 bit int
    uint16_t convertedValue = converterFunction(value);
    _currentSample.writeValue(convertedValue, offset);
}

Could you please provide some insights or guidance on how to resolve this issue? Any help would be greatly appreciated.

Thank you!

melkati commented 5 months ago

This is with:

https://github.com/Sensirion/arduino-upt-core.git#b2c0e76  ; Fix for arduino-ble-gadget incompatibly with arduino-upt-core library
https://github.com/Sensirion/arduino-ble-gadget.git#eb9d348

image

sdmueller commented 5 months ago

Hi @melkati

Thanks for the detailed investigation!

Just from quickly scanning through the code now, I am not sure where the problem lies. We will need to take some time to look into this issue properly.

I will update you when I know more.

melkati commented 5 months ago

Hi @sdmueller,

Thank you for your prompt response and for considering the issue we've reported.

We understand that investigating and resolving such issues can take time, so we appreciate your willingness to address this matter.

Please keep us updated on any progress or findings you may have while looking into the problem. We're here to provide any additional information that may be helpful in resolving this issue.

Thank you again for your attention and efforts in addressing this problem.

Best regards, Mario

LeonieFierz commented 5 months ago

Hi @melkati thank you again for investigating and reporting this issue. We found the reason for it, which was a wrong definition of the sample size in the Sensirion-UPT-Core library. The Bugfix-Version 0.3.1 of Sensirion-UPT-Core fixes the definition. You can also switch to use the sample type T_RH_CO2 instead of T_RH_CO2_ALT.

melkati commented 5 months ago

Hi @LeonieFierz,

Thank you for the update and for addressing the issue. I appreciate the effort put into investigating and fixing the problem.

I have already conducted a short preliminary test with the Bugfix-Version 0.3.1 of the Sensirion-UPT-Core library and switched to using the sample type T_RH_CO2 instead of T_RH_CO2_ALT. It seems that the problem with the data gaps has been resolved.

I will continue testing and provide further feedback.

Thank you again for your prompt response and support.

LeonieFierz commented 5 months ago

Thank you @melkati for your feedback. As the fix resolves the problem for you too I will close the issue.