Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Non blocking miThermometer.getData() #3

Open
Art-ut-Kia opened this issue Jan 25, 2023 · 19 comments
Open

Non blocking miThermometer.getData() #3

Art-ut-Kia opened this issue Jan 25, 2023 · 19 comments
Assignees
Labels
enhancement New feature or request

Comments

@Art-ut-Kia
Copy link

Hi, very happy to have found this library. I'd like to integrate several Mijia temp sensors in a home monitoring system.

Is your feature request related to a problem? Please describe.
The example (ESP32_ATC_MiThermometer_Client.ino) works well. However the miThermometer.getData() is blocking.
It's not a good news to have the ESP32 blocked for 5 seconds and not reacting to the user inputs

Describe the solution you'd like
A callback mechanism, integrated in the ATC_MiThermometer class, that would manage the sensor callbacks

  • semaphores to know whether new data is available or not

Describe alternatives you've considered
Make use of FreeRtos to launch getData in a seperate task. But I'm not sure it's compatible with NimBLEDevice.h/.cpp

Additional context
Ideally my system would include a touchscreen and implement a thermostat feature.
Blocking that for 5s is not acceptable

@matthias-bs matthias-bs added the enhancement New feature or request label Jan 25, 2023
@matthias-bs
Copy link
Owner

This makes sense! It may take a while until I find some spare time to work on it, though!

In any case, I recommend to use https://github.com/matthias-bs/ATC_MiThermometer instead of this repository.

@matthias-bs matthias-bs transferred this issue from matthias-bs/ESP32_ATC_MiThermometer_Library Jan 26, 2023
@matthias-bs
Copy link
Owner

Did you have a look at this?
https://github.com/theengs/decoder/blob/development/examples/ESP32/ScanAndDecode/ScanAndDecode.ino

It seems to be possible to set a "scan end callback".

@Art-ut-Kia
Copy link
Author

Thanx for your answers.
Meanwhile, I gave a try to FreeRTOS. I plugged it in your example from "https://github.com/matthias-bs/ATC_MiThermometer".
After tarnsferring all the code from loop() function into a FreeRTOS task, it worked fine and let the loop() function free for doing any required itterative stuff.
Thanx again for your job.

@matthias-bs
Copy link
Owner

Oh, that's good news! Would you mind providing your code as an example?

You can either fork the repository and make a pull request or attach your code to this issue and I will try to integrate it.

@Art-ut-Kia
Copy link
Author

Art-ut-Kia commented Jan 26, 2023

// https://github.com/matthias-bs/ATC_MiThermometer
// Mijia lywsd03mmc thermometers are first to be loaded with a custom firmware as per:
// https://pvvx.github.io/ATC_MiThermometer/TelinkMiFlasher.html

#include <FreeRTOS/FreeRTOS.h>
#include "ATC_MiThermometer.h"

// List of known sensors' BLE addresses
std::vector<std::string> knownBLEAddresses = {"a4:c1:38:02:8B:E0", "a4:c1:38:64:8F:10", "a4:c1:38:F3:C1:B4"};
ATC_MiThermometer miTh(knownBLEAddresses);

void setup() {
  pinMode(LED_BUILTIN, OUTPUT); digitalWrite(LED_BUILTIN, LOW);
  Serial.begin(115200);
  miTh.begin(); // initialization of Mijia sensors acquisition
  xTaskCreate(miThTask     ,   "miThTask",      10000,  NULL,        1,   NULL); // stack size: tried 1000, not sufficient
  //          task function, name of task, stack size, param, priority, handle
}

void loop() {
  digitalWrite(LED_BUILTIN, LOW);
  vTaskDelay(500 / portTICK_PERIOD_MS);
  digitalWrite(LED_BUILTIN, HIGH);
  vTaskDelay(500 / portTICK_PERIOD_MS);
}

void miThTask(void *pvParameters) {
  while (1) {
    miTh.resetData(); // Set sensor data invalid
    unsigned count = miTh.getData(5); // get sensor data (run BLE scan for 5 seconds)
    Serial.println("BLE Devices found (total): " + String(count));
    for (int i=0; i < miTh.data.size(); i++) {  
      if (miTh.data[i].valid) {
        MiThData_S* pMTD = &miTh.data[i]; // Mijia thermometer data pointer
        Serial.printf("sensor #: %d ; ", i);
        Serial.printf("temperature: %.2f°C ; ", pMTD->temperature *0.01f );
        Serial.printf("humidity: %.2f%% ; ",    pMTD->humidity    *0.01f );
        Serial.printf("voltage: %.3fV ; ",      pMTD->batt_voltage*0.001f);
        Serial.printf("battery charge: %d%% ; ",pMTD->batt_level);
        Serial.printf("RF power: %ddBm\n",      pMTD->rssi);
      }
    }
    miTh.clearScanResults(); // clear results from BLEScan buffer to release memory
    Serial.println();
    vTaskDelay(1000 / portTICK_PERIOD_MS);
  }
}

@Art-ut-Kia
Copy link
Author

I have a github account, but I'm not fluent at all with forks and pull requests.
So I let you integrate it.
To complete that and make the code usable, I will implement MutEx mechanisms to pass the sensors readings to other tasks (e.g. to the loop() function) rather than printing it on serial monitor.
I will also share the relevant code.

@matthias-bs
Copy link
Owner

Thank you very much in advance!

@Art-ut-Kia
Copy link
Author

Art-ut-Kia commented Jan 29, 2023

A little bit complicated, with a lot of data copies.
But it's the price to pay to render the sensors acquisition non blocking.
Still missing a semaphore mechanism to indicate that data were refreshed

// non-blocing Mijia lywsd03mmc thermometers acquisition
// based on this library:
// https://github.com/matthias-bs/ATC_MiThermometer
// Mijia lywsd03mmc thermometers are first to be loaded with a custom firmware as per:
// https://pvvx.github.io/ATC_MiThermometer/TelinkMiFlasher.html

#include "ATC_MiThermometer.h"

// List of known sensors' BLE addresses
std::vector<std::string> knownBLEAddresses = {"a4:c1:38:02:8B:E0", "a4:c1:38:64:8F:10", "a4:c1:38:F3:C1:B4"};
ATC_MiThermometer miTh(knownBLEAddresses);

MiThData_S miThData[3];
SemaphoreHandle_t mutex[3];

void setup() {
  Serial.begin(115200);
  for (int i=0; i<3; i++) mutex[i] = xSemaphoreCreateMutex();
  miTh.begin(); // initialization of Mijia sensors acquisition
  xTaskCreate(miThReadingTask,   "miThTask",      10000,  NULL,        1,   NULL); // stack size: tried 1000, not sufficient
  //            task function, name of task, stack size, param, priority, handle
}

void loop() {
  int count = 0;
  for (int i=0; i < 3; i++) {

    // gets a local copy of the ith sensor reading
    // mutex protection ensures reading struct integrity
    MiThData_S miThDat;
    if (xSemaphoreTake(mutex[i], (TickType_t)10)==pdTRUE) {
      miThDat = miThData[i];
      xSemaphoreGive(mutex[i]);
    } else miThDat.valid = false;

    // displays sensor # and sensor reading if valid
    if (miThDat.valid) {
      count++;
      Serial.printf("sensor #%d -> ", i);
      Serial.printf("temperature: %.2fC ; ",   miThDat.temperature *0.01f );
      Serial.printf("humidity: %.2f%% ; ",     miThDat.humidity    *0.01f );
      Serial.printf("voltage: %.3fV ; ",       miThDat.batt_voltage*0.001f);
      Serial.printf("battery charge: %d%% ; ", miThDat.batt_level);
      Serial.printf("RF power: %ddBm\n",       miThDat.rssi);
    }
  }
  if (count) Serial.println();
  vTaskDelay(6000 / portTICK_PERIOD_MS);
}

void miThReadingTask(void *pvParameters) {
  while (1) {
    miTh.resetData(); // Set sensor data invalid
    miTh.getData(5); // get sensor data (run BLE scan for 5 seconds)
    // makes a copy of each sensor reading under mutex protection
    for (int i=0; i < 3; i++) if (xSemaphoreTake(mutex[i], (TickType_t)10)==pdTRUE) {
      miThData[i] = miTh.data[i];
      xSemaphoreGive(mutex[i]);
    } else miThData[i].valid = false;
    miTh.clearScanResults(); // clear results from BLEScan buffer to release memory
    vTaskDelay(1000 / portTICK_PERIOD_MS);
  }
}

@matthias-bs
Copy link
Owner

matthias-bs commented Jan 29, 2023

Hi,

thank you very much! I added it to the examples directory.

The CI run (see https://github.com/matthias-bs/ATC_MiThermometer/actions) gives me a compile error due to FreeRTOS missing:

Building ATC_MiThermometer_Nonblocking.ino ... 
/home/runner/work/ATC_MiThermometer/ATC_MiThermometer/examples/ATC_MiThermometer_Nonblocking/ATC_MiThermometer_Nonblocking.ino:9:10: fatal error: FreeRTOS/FreeRTOS.h: No such file or directory
 #include <FreeRTOS/FreeRTOS.h>
          ^~~~~~~~~~~~~~~~~~~~~
compilation terminated. 

Where can I find it to include it in the build run?

Thanks in advance!

Matthias

@matthias-bs
Copy link
Owner

matthias-bs commented Jan 29, 2023

O.k., it works if I comment out this #include directive.
Seems to be part of the Arduino-ESP32 environment.

I also had to add the following for the generic esp32:esp32:esp32 target:

#ifndef LED_BUILTIN
#define LED_BUILTIN 13
#endif

Which target have you been using?

@matthias-bs matthias-bs self-assigned this Jan 29, 2023
@matthias-bs
Copy link
Owner

Please let me know when you think you have a stable version; I will create a new release then.

@Art-ut-Kia
Copy link
Author

The FreeRTOS library can be downloaded from Arduino IDE as can be seen in the following screenshot

image

@Art-ut-Kia
Copy link
Author

I am configuring my Arduino IDE (2.0.3) for the board Wemos R1 D32.
I had to explicitely download the FreeRTOS library.
Regarding the LED_BUILTIN it's declared somewhere for the Wemos R1 D32.
Take care that for the Wemos R1 D32 board, the built-in LED is not on pin 13

@Art-ut-Kia
Copy link
Author

Please let me know when you think you have a stable version; I will create a new release then.

Sure.

@matthias-bs
Copy link
Owner

The FreeRTOS library can be downloaded from Arduino IDE as can be seen in the following screenshot

image

Hmmm... I could not find it in Arduino IDE 1.8.19. Now I've updated to 2.0.3. The sketch still compiles without installing FreeRTOS explicitely. And the documentation for this library says it's for AVR targets. I'm not sure if this is actually used for the ESP32.

@Art-ut-Kia
Copy link
Author

You are right. I removed the #include and it works the same.
Obviously it load the right library depending on the selected board.

image

@matthias-bs
Copy link
Owner

Is there anything to be done or can you close this issue?

@bazooka02
Copy link

Hi @matthias-bs,

I would like to extend it a bit:
There are a few places in this example, where you could find "magic numbers".
I think it would be better if you could Manage it form one numeric constant, makes it easier to adopt into your case.

e.g:
const int Sensors = 9;
.
MiThData_S miThData[Sensors];
SemaphoreHandle_t mutex[Sensors];
.
for (int i = 0; i < Sensors; i++) mutex[i] = xSemaphoreCreateMutex();
.
for (int i = 0; i < Sensors; i++) {
.
for (int i = 0; i < Sensors; i++)

What do you think?

+1: I also created a Struct to be able to bind together the Name of the sensor and the Mac address.

@matthias-bs
Copy link
Owner

Hi @bazooka02,

That sounds good! Please feel free to implement the changes and make a pull request!

Regards
Matthias

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants