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

Sometimes CO2 value prints to wrong place on ESP32 display #145

Closed
melkati opened this issue Jan 28, 2024 · 12 comments
Closed

Sometimes CO2 value prints to wrong place on ESP32 display #145

melkati opened this issue Jan 28, 2024 · 12 comments
Assignees
Labels
bug Something isn't working

Comments

@melkati
Copy link
Owner

melkati commented Jan 28, 2024

It doesn't happen on T-Display S3.

I tried reducing font 90 to 88 but did not fix it.

It never happened to @Coscolin with same version and hardware as me and nobody else reported this.

image

@melkati
Copy link
Owner Author

melkati commented Feb 4, 2024

Looks like it has something to do with the web server.
I can see artifacts on the display when I reload the preferences web page.

@melkati melkati reopened this Feb 4, 2024
@melkati melkati added the bug Something isn't working label Feb 4, 2024
@melkati melkati self-assigned this Feb 4, 2024
@melkati melkati moved this to Low priority in CO2 Gadget Bugs and issues Feb 5, 2024
Coscolin added a commit to Coscolin/CO2-Gadget that referenced this issue Feb 7, 2024
@Coscolin
Copy link
Contributor

Coscolin commented Feb 7, 2024

Last night I started to investigate the bug.

My first step was to add a trace each time the CO2 value was displayed on the screen. My surprise was that it was repainted a thousand times in a row, even if the value had not changed.

I created the variable co2_displayed to store the value displayed on the screen and added the condition to the ShowCO2 loop so that it is only executed when co2 != co2_displayed

When entering the menu you have to set co2_displayed = 0 to display the value when exiting.

By doing this the bug has disappeared !!!

I think the same should be implemented for the rest of the values: temperature, humidity, battery, etc.

The less the processor has to do, the less it will consume.

@melkati
Copy link
Owner Author

melkati commented Feb 7, 2024

Thank you, Sergio.

It can be a good workaround but I think it is not the root cause of the issue.

The issue looks related to the web server #145 (comment). Since I wrote that another user reported the same.

Also, it looks like it doesn't happen os ESP32 S3, so it may be memory related.

Under the hood CO2 Gadget only prints on screen what is needed and when is needed, I think there is no need to save on display prints (it does not affect power consumption either).

Anyway, it can be a good workaround until we find the root cause. I was thinking on redrawing the screen after a request to the web server (this will fix any issue related to this) or your fix (will only fix this case and needs a lot of new code - and more as we continue including things as the PM sensors and more data to show). I don't know yet witch one to implement... 🤔

With this fix the issue can look like it's fixed but only because it is more difficult to coincide a web request with a display update. If it coincides this will not solve the underlying issue and the screen will be corrupted).

We have time until the next release to decide...

@melkati melkati changed the title Sometimes CO2 value prints to wrong place on TTGO T-Display Sometimes CO2 value prints to wrong place on ESP32 display Feb 7, 2024
@melkati
Copy link
Owner Author

melkati commented Feb 7, 2024

I just discovered it also happens on the ST7789_240x320 variant.

Looks to be worse. It suggests a memory problem as it leaves less free memory (having a display with more resolution it uses more memory).

It doesn't even load the page preferences.html.

I would like to avoid changing partitions, while possible, for backwards compatibility.

@melkati melkati moved this from Low priority to High priority in CO2 Gadget Bugs and issues Feb 7, 2024
@Coscolin
Copy link
Contributor

Coscolin commented Feb 7, 2024

Thank you, Sergio.

It can be a good workaround but I think it is not the root cause of the issue.

The issue looks related to the web server #145 (comment). Since I wrote that another user reported the same.

Also, it looks like it doesn't happen os ESP32 S3, so it may be memory related.

Under the hood CO2 Gadget only prints on screen what is needed and when is needed, I think there is no need to save on display prints (it does not affect power consumption either).

Anyway, it can be a good workaround until we find the root cause. I was thinking on redrawing the screen after a request to the web server (this will fix any issue related to this) or your fix (will only fix this case and needs a lot of new code - and more as we continue including things as the PM sensors and more data to show). I don't know yet witch one to implement... 🤔

With this fix the issue can look like it's fixed but only because it is more difficult to coincide a web request with a display update. If it coincides this will not solve the underlying issue and the screen will be corrupted).

We have time until the next release to decide...

You say that "Under the hood CO2 Gadget only prints on screen what is needed and when is needed" but when I traced showCO2() function yesterday is executed in every main loop().

Yesterday also I detected that bug only occurs when internal web server load a page. If you reload webpage bug is reproduced.

Enter and exit menu to reload display, refresh webpage and bug is showed again.

I hope you can find the root of bug.

@melkati
Copy link
Owner Author

melkati commented Feb 7, 2024

Thank you, Sergio.
It can be a good workaround but I think it is not the root cause of the issue.
The issue looks related to the web server #145 (comment). Since I wrote that another user reported the same.
Also, it looks like it doesn't happen os ESP32 S3, so it may be memory related.
Under the hood CO2 Gadget only prints on screen what is needed and when is needed, I think there is no need to save on display prints (it does not affect power consumption either).
Anyway, it can be a good workaround until we find the root cause. I was thinking on redrawing the screen after a request to the web server (this will fix any issue related to this) or your fix (will only fix this case and needs a lot of new code - and more as we continue including things as the PM sensors and more data to show). I don't know yet witch one to implement... 🤔
With this fix the issue can look like it's fixed but only because it is more difficult to coincide a web request with a display update. If it coincides this will not solve the underlying issue and the screen will be corrupted).
We have time until the next release to decide...

You say that "Under the hood CO2 Gadget only prints on screen what is needed and when is needed" but when I traced showCO2() function yesterday is executed in every main loop().

Yesterday also I detected that bug only occurs when internal web server load a page. If you reload webpage bug is reproduced.

Everything in CO2 Gadget is executed always in the loop. It's intended that way, as it's completely event-driven.

What I mean is that CO2 Gadgets uses sprites so it only updates the "live" parts of the display. This will improv with time as not everything is converted to sprites yet.

If you include this code to measure the time it takes for a block of code to execute:

// To use the function for measuring the execution time of another function, call `measureFunctionTime(true)` 
// before the function you want to measure and `measureFunctionTime(false)` after it. This will print the elapsed time in milliseconds.
void measureFunctionTime(bool before) {
    static unsigned long startMillis;  // Variable to store the start time of the measurement
    if (before) {
        // Store the current time before calling the function
        startMillis = millis();
    } else {
        // Calculate the elapsed time after calling the function
        unsigned long elapsedTime = millis() - startMillis;
        Serial.print("The function took: ");
        Serial.print(elapsedTime);
        Serial.println(" milliseconds");
    }
}

And you call it on the display loop as:

void displayShowValues() {
    uint8_t currentDatum = tft.getTextDatum();
    measureFunctionTime(true);
    showCO2(co2, elementPosition.co2X, elementPosition.co2Y);
    showCO2units(elementPosition.co2UnitsX, elementPosition.co2UnitsY);
    showTemperature(temp, elementPosition.tempX, elementPosition.tempY);
    showHumidity(hum, elementPosition.humidityX, elementPosition.humidityY);
    showBatteryIcon(elementPosition.batteryIconX, elementPosition.batteryIconY);
    showBatteryVoltage(elementPosition.batteryVoltageX, elementPosition.batteryVoltageY);
    showWiFiIcon(elementPosition.wifiIconX, elementPosition.wifiIconY);
    showMQTTIcon(elementPosition.mqttIconX, elementPosition.mqttIconY);
    showBLEIcon(elementPosition.bleIconX, elementPosition.bleIconY);
    showEspNowIcon(elementPosition.espNowIconX, elementPosition.espNowIconY);
    measureFunctionTime(false);

    // Revert the datum setting
    tft.setTextDatum(currentDatum);
    tft.setTextSize(2);
}

You will find out that it takes just 72ms to update the display:

14:50:38.490 > The function took: 72 milliseconds
14:50:38.573 > The function took: 72 milliseconds
14:50:38.656 > The function took: 73 milliseconds
14:50:38.739 > The function took: 72 milliseconds

Yes, we can optimize even more the display updating but, in my opinion, it's not needed right now as it will need a lot of code for no obvious benefit.

Yesterday also I detected that bug only occurs when internal web server load a page. If you reload webpage bug is reproduced.

That is what I just told you:

The issue looks related to the web server #145 (comment). Since I wrote that another user reported the same.

Maybe I should do some testing with optimized partitions to test if the issue goes away. I'm still yest to see this happening in a ESP32 S3 with different partitions that leaves a lot more free memory... The issue with changing partitions is that we lose backwards compatibility and users will have to configure CO2 Gadget again from scratch (maybe we can add a backup and restore for settings in the preferences web page (already planned).

@melkati
Copy link
Owner Author

melkati commented Feb 9, 2024

Finally, I found out the cause of this bug!

It's a memory problem where, if CO2 Gadget sending a web request (the web server is very memory hungry), there is not memory left to create the sprite, so printing defaults to 0, 0 (upper left corner).

You can test it with this modified version of the showCO2() function:

void showCO2(uint16_t co2, int32_t posX, int32_t posY) {
    if (co2 > 9999) co2 = 9999;

    spr.loadFont(BIG_FONT);
    uint16_t width = spr.textWidth("0000");
    uint16_t height = elementPosition.co2FontDigitsHeight;
    uint16_t posSpriteX = posX - width;
    uint16_t posSpriteY = posY - height;
    if (posSpriteX < 0) posSpriteX = 0;
    if (posSpriteY < 0) posSpriteY = 0;
    if (spr.createSprite(width, height) == nullptr) {
        Serial.printf("-->[TFT ] Error: sprite not created, not enough free RAM! Free RAM: %d\n", ESP.getFreeHeap());
        spr.unloadFont();
        spr.deleteSprite();
        return;
    }
    spr.setTextColor(getCO2Color(co2), TFT_BLACK);
    spr.setTextDatum(TR_DATUM);
    spr.drawNumber(co2, width, 0);
    spr.pushSprite(posSpriteX, posSpriteY);
    spr.unloadFont();
    spr.deleteSprite();
}

Then if you request a web page from the web server, as /preferences.html, you can see in the log:

12:06:31.299 > -->[TFT ] Error: sprite not created, not enough free RAM! Free RAM: 47204
12:06:31.330 > -->[TFT ] Error: sprite not created, not enough free RAM! Free RAM: 47204
12:06:31.363 > -->[TFT ] Error: sprite not created, not enough free RAM! Free RAM: 48428
12:06:31.396 > -->[TFT ] Error: sprite not created, not enough free RAM! Free RAM: 45556
12:06:31.428 > -->[TFT ] Error: sprite not created, not enough free RAM! Free RAM: 43232
12:06:31.457 > -->[TFT ] Error: sprite not created, not enough free RAM! Free RAM: 52116

Now I have to investigate why the web server is so memory hungry and why the sprite is not created (as 40-50KB should be more than enough to create the sprite, I guess).

It will continue...

@melkati
Copy link
Owner Author

melkati commented Feb 9, 2024

I just uploaded the fix to the development branch.

I plan to have a new release version very soon as it was a stopper to it.

@melkati
Copy link
Owner Author

melkati commented Feb 9, 2024

New BETA version of CO2 Gadget v0.9.030

The new beta version of CO2 Gadget is now available with important new features and some fixes.

As important new features to highlight:

✔️ Support for buzzer. Configurable in number of beeps, time between beeps. repetitions, etc.
✔️ Support for a 240x320 screen with ST7789 driver

In addition, among the fixes that have been implemented, the following stand out:

✔️ Corrects the screen that "got dirty" sometimes.
✔️ Correct the temperature offset.
✔️ Corrects the regulation and management of the screen brightness of the TTGO T-Display boards.

It's available to test on TTGO T-Display boards (normal and sandwich ) at:

https://emariete.com/en/co2-meter-gadget/#CO2_Gadget_Version_Beta-Desarrollo

Feedback will be greatly appreciated. 😉

@Olaruci
Copy link

Olaruci commented Feb 10, 2024

IMG_20240210_092014_050.jpg

It is fixed but it always has a 0 in front for values below 1000.

@melkati
Copy link
Owner Author

melkati commented Feb 10, 2024

I rewrote the function that prints the CO2 value to the display and probably oversight it.

I will modify this.

Thank you very much for reporting.

@melkati
Copy link
Owner Author

melkati commented Feb 11, 2024

The fix is implemented in the last development branch code c5728b0.

I'll include it in the next binary (easy web install) soon. Check for version > v0.9.32

@melkati melkati closed this as completed Feb 18, 2024
@github-project-automation github-project-automation bot moved this from High priority to Closed in CO2 Gadget Bugs and issues Feb 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Development

No branches or pull requests

3 participants