znanev / ATC_MiThermometer

Custom firmware for the Xiaomi Thermometer MHO-C401 and Telink Flasher via USB to Serial converter
2 stars 0 forks source link

Energy usage while updating display #7

Closed michapr closed 3 years ago

michapr commented 3 years ago

Original firmware while updating display: c401_partial_update (take y-value/10 for mA)

Custom firmware with cpu_stall_wakeup_by_timer0() c401_cust_fw_2 (one time unit = 500ms)

This part comes up only while updating display with new value, any idea what can be different here? (100ms <-> 1.5sec)

znanev commented 3 years ago

Checking the original XiaomiLYWSD03MMC showed that it was not possible to use it. Large errors in temperature and humidity.

@pvvx What sensor did you compare that to? From what I can tell, all 5 Xiaomi sensors (2 x LYWSD02, 2 x MHO-C401 and one MHO-C303) in the same room agree on their values, so at least they are consistent and a simple factor could be used to compensate for temperature / humidity value, if needed.

pvvx commented 3 years ago

@znanev The wife spoke about the instability of the data readings. The effect is that the temperature changes and the reading freezes indefinitely when it drops. Some external conditions have not changed yet. It looks like the effect of the sensor falling off. Installed in an unheated room - unstable temperature from +10 to + 22C, if the door is open. She checked against an ordinary mercury thermometer. I don't think that a mercury thermometer has better performance. :) This is the first thing. But there is no precision itself. It is possible that at 20 + -5C it shows correctly, but in other ranges it is not. I have almost all types of temperature and humidity sensors. PS: I am writing via google translator - there may be incorrect descriptions.

pvvx commented 3 years ago

SHT85 - the best cheap on the market - does not change reading when polled. It does not heat up when active. The rest of the sensors depend on the number of polls and the current consumption during active measurement. Reaches +3 C. By humidity. Condensation occurs in many sensors when the temperature changes and they give incorrect readings until they get hot. Applies to BME / BMP 280 - iron case with hole :) When turned on, it gives different temperature readings as it warms up. Up to +2 C. No built-in compensation from own heating.

Sample: https://raw.githubusercontent.com/pvvx/UBIA/master/DOCs/img/testBMP280.gif

This is the reason for "global warming". :) "global warming" with the introduction of electronic sensors: https://upload.wikimedia.org/wikipedia/commons/thumb/d/db/Global_Temperature_And_Forces.svg/300px-Global_Temperature_And_Forces.svg.png

michapr commented 3 years ago

@znanev

@michapr Can I please ask you to pull the code from branch test-pin-wakeup, build it and test it on your device?

I have tested - energy usage is much better - but display does not work more ;)

Display is black with some white elements, they are changing when temperature will be changed, so something happen at display. I could flash back via OTA - so you can try yourself without risk :)

pvvx commented 3 years ago

https://github.com/znanev/ATC_MiThermometer/blob/master/ATC_Thermometer/app.c#L85 overflow: 5 60000 CLOCK_SYS_CLOCK_1MS = 5 60000 24000000 / 1000 = 7200000000 = 0x1 AD27 4800

znanev commented 3 years ago

https://github.com/znanev/ATC_MiThermometer/blob/master/ATC_Thermometer/app.c#L85 overflow: 5 60000 CLOCK_SYS_CLOCK_1MS = 5 60000 24000000 / 1000 = 7200000000 = 0x1 AD27 4800

  • Hundreds of warnings if gcc -pedantic

Indeed it overflows, I noticed it, but haven't have the time nor incentive to change it yet, as it's "inherited" from atc1441's codebase ;)

My immediate goal is to make the EPD work like in the original firmware. Optimisations will follow after that, for sure :)

michapr commented 3 years ago

@znanev I agree, here in this task we should try work at the EPD at first ;)

@pvvx Some other optimizations (like this overflow) are already done in personal codebases - anyway thank you for reminder ;)

znanev commented 3 years ago

@znanev

@michapr Can I please ask you to pull the code from branch test-pin-wakeup, build it and test it on your device?

I have tested - energy usage is much better - but display does not work more ;)

Display is black with some white elements, they are changing when temperature will be changed, so something happen at display. I could flash back via OTA - so you can try yourself without risk :)

So it seems that the effect is the same as in #8. In that (bad) release, the only change was the sleep mode - instead of waiting in idle state:

cpu_stall_wakeup_by_timer0(microseconds * CLOCK_SYS_CLOCK_1US);

the CPU was waiting in deep sleep state:

cpu_sleep_wakeup(DEEPSLEEP_MODE_RET_SRAM_LOW32K, PM_WAKEUP_TIMER, clock_time() + microseconds * CLOCK_SYS_CLOCK_1US);

So from functional perspective, it appears that now

cpu_set_gpio_wakeup(IO_BUSY_N, Level_High, 1);

behaves the same as the deep sleep waiting.

It should be something related to recovering from deep sleep, but I have no idea at this point. @pvvx - do you have any clue about this?

znanev commented 3 years ago

But there is no precision itself. It is possible that at 20 + -5C it shows correctly, but in other ranges it is not.

This might be the key! All my sensors so far have been on room temperature, from 18C to 30C and their readings have generally been in the ballpark of a simple spirit thermometer. But I haven't tested them (yet) with extreme temperatures - like close to freezing point or more than 30C. So far I'm fine with my sensors, can't complain. From what I know, MHO-C401 uses Swiss made sensors - Sensirion SHT30-DIS, so these in my opinion are pretty accurate.

PS: I am writing via google translator - there may be incorrect descriptions.

No worries, I understand everything that you write so far. If there is something that I can't understand, I'll surely ask you to rephrase ;)

znanev commented 3 years ago

This is the reason for "global warming". :) "global warming" with the introduction of electronic sensors: https://upload.wikimedia.org/wikipedia/commons/thumb/d/db/Global_Temperature_And_Forces.svg/300px-Global_Temperature_And_Forces.svg.png

@pvvx Nice catch, hehe 😄 So even just measuring the temperature with those sensors, warming them up by 2 degrees, contributes to the global warming too 😛

pvvx commented 3 years ago

Doesn't work in the refrigerator. Shows 'L'

pvvx commented 3 years ago

https://github.com/znanev/ATC_MiThermometer/blob/master/ATC_Thermometer/flash.c#L12

    flash_write_page(0x78000, 1, (unsigned char *)&read);
znanev commented 3 years ago

Doesn't work in the refrigerator. Shows 'L'

L probably stand for Low. Fair enough, it seems that those sensors are suitable for room temperatures only, not outside or inside a fridge / freezer 😄

znanev commented 3 years ago

https://github.com/znanev/ATC_MiThermometer/blob/master/ATC_Thermometer/flash.c#L12

  flash_write_page(0x78000, 1, (unsigned char *)&read);

I haven't touched this code, yet. What do you want to point about this function call?

pvvx commented 3 years ago

LCD work at -20C, which is strange

znanev commented 3 years ago

EPD however won't work below 0C - its fluids will freeze!

pvvx commented 3 years ago
flash_write_page(0x78000, 1, read);

= flash_write_page(0x78000, 1, 0) flash_write_page declared as:

void flash_write_page(unsigned long addr, unsigned long len, unsigned char *buf)
src/flash.c: In function 'erase_mi_data':
src/flash.c:12:2: warning: passing argument 3 of 'flash_write_page' makes pointer from integer without a cast
E:/Telink/825x/Telink_825X_SDK/components/drivers/8258/flash.h:79:27: note: expected 'unsigned char *' but argument is of type 'uint8_t'
znanev commented 3 years ago

Guys, do you mind starting a "discussion" here:

https://github.com/znanev/ATC_MiThermometer/discussions

It will probably be easier to catch up there.

znanev commented 3 years ago
flash_write_page(0x78000, 1, read);

= flash_write_page(0x78000, 1, 0) flash_write_page declared as:

void flash_write_page(unsigned long addr, unsigned long len, unsigned char *buf)

It will be better to hint the original code maintainer - @atc1441 for that. I haven't touched / used that functionality yet.

znanev commented 3 years ago

I have a theory why changing the idle wait for deep sleep when waiting for BUSY_N to be de-asserted bugs the display.

Using idle wait doesn't allow the CPU to go into sleep, so the code on start-up - in function user_init_normal, which in turn invokes function epd_init(&c, 1), is executed without interruption.

So when changing the idle wait for deep sleep, the first wakeup (and in fact all subsequent) of the CPU following the wait for BUSY_N goes into function user_init_deepRetn, which messes up the code in epd_init function. What happens is, only the first sequence from the redraw condition is executed:

https://github.com/znanev/ATC_MiThermometer/blob/e0069a286140b7af7a8d964cbf7d4b74eda49924/ATC_Thermometer/epd.c#L256

This sequence sends initialisation waveforms for the black segments. After it is transmitted, the CPU waits for BUSY_N to become High again. And in the case of deep sleep, the redraw condition is false and the seconds sequence, initialising the waveforms for the white segments, won't be executed:

https://github.com/znanev/ATC_MiThermometer/blob/e0069a286140b7af7a8d964cbf7d4b74eda49924/ATC_Thermometer/epd.c#L271

Thinking forward, it appears that EPD driver should be completely rewritten as a state machine. Because after every wait for BUSY_N, resuming will begin the execution of the program from the beginning (in function user_init_deepRetn in app.c).

Until recently, my other goal was to keep the structure of the EPD driver as close to the "original" C++ code as possible:

https://github.com/znanev/MHO-C401

All was fine while I used the EPD alone, connected to another MCU - ESP8266 in that instance. But minimum power consumption was never in my requirements list so to say. So going forward, I think that the following should happen:

  1. Complete break from the original EPD driver (i.e. compatibility not to be considered when doing modifications).
  2. Think about how to re-implement sending each EPD command sequence as a state machine (because after every wake up following wait for BUSY_N, the program execution will in fact start from the beginning).
  3. Optimise some of the sequences, bringing them in line with those from the original firmware (using the new logic analyser captures).

I see myself working on 1 and 3 in the near future. Any volunteers for 2? 😉

pvvx commented 3 years ago

StartUp Original Xiaomi LYWSD03MMC image

pvvx commented 3 years ago

Work cycle (not connected): image Interval 1690.4 ms average consumption 19.22 uA (3.3V)

znanev commented 3 years ago

Work cycle (not connected): image Interval 1690.4 ms average consumption 19.22 uA (3.3V)

@pvvx Is this for MHO-C401? If not, we are comparing oranges and apples, because LYWSD03MMC uses LCD, whereas MHO-C401 used EPD.

pvvx commented 3 years ago

@pvvx Is this for MHO-C401? If not, we are comparing oranges and apples, because LYWSD03MMC uses LCD, whereas MHO-C401 used EPD.

I don't think there will be much difference.

LYWSD03MMC deep-sleep ~6 uA.

znanev commented 3 years ago

I don't think there will be much difference.

I may be wrong, but charging the charge pump of the EPD maybe consumes more energy compared to energising the LCD segments... But as you say, the difference might not be significant

michapr commented 3 years ago

@znanev

So when changing the idle wait for deep sleep, the first wakeup (and in fact all subsequent) of the CPU following the wait for BUSY_N goes into function user_init_deepRetn, which messes up the code in epd_init function. What happens is, only the first sequence from the redraw condition is executed:

Maybe we could use a flag to detect - we are within the send_sequence() function or not? And depending on this execute the needed functions in epd_init() ?

pvvx commented 3 years ago

The following charts alternate: image Reading temperature and humidity sensor (?): image To clarify what causes the 600 μA consumption, we need to solder the I2C pin ...

The diagram shows that the wake interrupt is charging. On waking up, after the start of the chip, when the program reaches the operation, the current of 600 uA disappears.

pvvx commented 3 years ago

The connection to "nRF Connect" is terrible. In half an hour, it will drain the battery ... It needs to be redone. image image

michapr commented 3 years ago

@pvvx

The connection to "nRF Connect" is terrible.

Yes, the main goal of the ATC firmware for MMC3 is to get the values without connection, because of battery drain. So all values will be send in advertising package. I have seen the same some time ago - but because do not need the connection, it was not important for me in his time.

But here - in this issue we try to solve at first the EPD integration... Maybe you can have a look at the epd.c and give us some hints and help?

All the other issues would be very helpful to add in discussion - or in new issues, it will be better to sort it out then... All what you say is very helpful to optimize the sensor - but this are different issues.we cannot work on all issues at same time :) (BTW: I have all noted,... but... )

Thank you for your work and help!! 👍

znanev commented 3 years ago

I propose to stick to the EPD-related issues in this fork. The original codebase is something that I'm not very interested to dig into at present. It will be better if the original code is optimised first, later on we can merge the relevant parts here.

pvvx commented 3 years ago

This is an analogue of the ATC_Thermometer firmware. image There is still work to be done before EPD.

michapr commented 3 years ago

@znanev If I understand right we do not need all the epd_init() within the send_sequence(), right? I just excluded it using a RAM bool flag (epd_active true/false - set in the send_sequence() ) but no changes - display is black. What I understood wrong?

What I have seen, adding a sleep_ms(100) at the end (after last cpu_set_gpio_wakeup() ) - display will show data (with bad contrast and some old grey segments,...) - why this?

michapr commented 3 years ago

@pvvx

There is still work to be done before EPD.

YES - you are right !!! But could you please report this on the main repo at ATC? With new issues? We are there also active and working there too.

But here in the forked repo we try to optimize the EPD driver only ;)

pvvx commented 3 years ago

I propose to stick to the EPD-related issues in this fork. The original codebase is something that I'm not very interested to dig into at present. It will be better if the original code is optimised first, later on we can merge the relevant parts here.

For EPD, you need to prepare a deep sleep state machine. If the EPD control codes are correct, then you need to rewrite them under the state machine.

znanev commented 3 years ago

@znanev If I understand right we do not need all the epd_init() within the send_sequence(), right? I just excluded it using a RAM bool flag (epd_active true/false - set in the send_sequence() ) but no changes - display is black. What I understood wrong?

What I have seen, adding a sleep_ms(100) at the end (after last cpu_set_gpio_wakeup() ) - display will show data (with bad contrast and some old grey segments,...) - why this?

The issue is not only within the epd_init() function, but send_sequence() itself - after every BUSY_N wake up from deep sleep, the MCU will start the program from "scratch" - only "remembering" the annotated RAM variables. That's why a FSM (finite state machine) is needed - defining the transitions and sequences of the EPD commands. I'll think about some crude implementation later on.

pvvx commented 3 years ago

After init_ble() use the bls_pm_registerFuncBeforeSuspend() function

void bls_pm_registerFuncBeforeSuspend (suspend_handler_t func );
int app_suspend_enter ()
{
    if (stop_sleep)
    {
        bls_pm_setSuspendMask(SUSPEND_DISABLE);
        return 0;
    }
    return 1;
}

bls_pm_registerFuncBeforeSuspend( &app_suspend_enter );
pvvx commented 3 years ago
    bls_app_registerEventCallback(BLT_EV_FLAG_SUSPEND_ENTER, &func_suspend_enter);
    bls_app_registerEventCallback(BLT_EV_FLAG_SUSPEND_EXIT, &func_suspend_exit);
    bls_app_registerEventCallback(BLT_EV_FLAG_GPIO_EARLY_WAKEUP, &func_gpio_wakeup);

or

    blc_hci_registerControllerEventHandler(event_handler);      //register event callback
    int event_handler(u32 h, u8 *p, int n) {
        if (h &HCI_FLAG_EVENT_BT_STD) {     //ble controller hci event
            u8 evtCode = h & 0xff;
            if(evtCode == HCI_EVT_DISCONNECTION_COMPLETE)  //connection terminate
            {
            }
            else if(evtCode == HCI_EVT_ENCRYPTION_CHANGE)
            {
            }
            ....
        }
        if (h &HCI_FLAG_EVENT_TLK_MODULE) {
            switch((u8)(h&0xff))
            case BLT_EV_FLAG_GPIO_EARLY_WAKEUP:
                break;
            case BLT_EV_FLAG_SUSPEND_ENTER:
                break;
            case BLT_EV_FLAG_SUSPEND_EXIT:
                break;
            ....    
        }
    }
znanev commented 3 years ago

What I see is that BUSY_N have in original FW some additional short peaks - looking at our code I thought it should be here too. We are waiting multiple times for the BUSY_N state, right?

@michapr Just checked the original firmware captures, BUSY_N is asserted (pulled low by the EPD) following one of these 3 commands:

The epd.c driver awaits for BUSY_N signal to be pulled back high for those 3 commands above too. I see that I've put waiting for BUSY_N for these two commands too:

Which serves no purpose, I'll remove these two waits.

pvvx commented 3 years ago
wakeup_callback_t wakeup_cb; // typedef void (*wakeup_callback_t)(void);

...
wakeup_cb =  read_sensor_cb;
...

_attribute_ram_code_ void read_sensor_cb(void) {
    wakeup_cb = NULL;
    ....
}

_attribute_ram_code_ void blt_pm_proc(void) {
    if(ota_is_working){
        bls_pm_setSuspendMask(SUSPEND_DISABLE);
        bls_pm_setManualLatency(0);
    }else{
        if (wakeup_cb) {
            if(((bls_pm_getSystemWakeupTick() - clock_time())) > 30 * CLOCK_16M_SYS_TIMER_CLK_1MS) {
                cpu_set_gpio_wakeup(GPIO_WAKEUP_SENS, Level_High, 1);  // pad high wakeup deepsleep
                bls_pm_setWakeupSource(PM_WAKEUP_PAD);  // gpio pad wakeup suspend/deepsleep
            } else ...
        }
        bls_pm_setSuspendMask (SUSPEND_ADV | DEEPSLEEP_RETENTION_ADV | SUSPEND_CONN | DEEPSLEEP_RETENTION_CONN);
    }
}

attribute_ram_code_ void user_init_deepRetn(void){ // after sleep this will get executed
    blc_ll_initBasicMCU();
    rf_set_power_level_index(RF_POWER_P3p01dBm);
    blc_ll_recoverDeepRetention();

    if(pm_is_deepPadWakeup()) {
        if (wakeup_cb)  wakeup_cb();
    }
}

image

znanev commented 3 years ago

@pvvx I'm not sure I can follow up your thought process here. Can you please expand your examples with some explanations?

pvvx commented 3 years ago

In the main loop (main_loop), assign calback (). At the end of the main_loop() , blt_pm_proc () is called. It will set the modes for the sleep mode. Then the MCU falls asleep and wakes up on the event specified: cpu_set_gpio_wakeup (GPIO_WAKEUP_SENS, Level_High, 1); // deep sleep for deep awakening. On wake-up (in main.c) after deep-sleep, the user_init_deepRetn() routine is executed. In it you can see that the activation came from the GPIO with the pm_is_deepPadWakeup (). ... and execute calback().

if (((bls_pm_getSystemWakeupTick () - clock_time ()))> 30 * CLOCK_16M_SYS_TIMER_CLK_1MS) This is a check - what is the next pause for the BLE machine. If less, then there is no point. It's easier to set flag and next wait for the next wake up of the BLE machine.

This is probably the simplest example... Next, you need to enter your codes EPD.

znanev commented 3 years ago

@pvvx Thank you for your thorough explanation of the process above. So it seems that Telink's SDK has already implemented the hardest part - provide the plumbing for a state machine.

So, this brings the horizon of energy-efficient EPD driver rewrite a bit closer :)

I'll start with some code clean-up first, until I wrap my head around the implementation details of the state machine driver :)

pvvx commented 3 years ago

The SDK has "int event_handler (u32 h, u8 * para, int n)" for all events. For everyone events.

Just remember that you need to initialize all hardware before using it. And also off / close after use. This is BLE specific... Most of the equipment will turn off the chip itself when going into deep sleep. But before this event, it will consume extra energy.

znanev commented 3 years ago

I wish we had the English version of the BLE_SDK_Developer_Handbook.pdf document. I can't make sense from most of the Google translated sentences... There are quite nice examples in there, along with explanations how to use different BLE-related functions. But my Chinese is non-existent, so I'll rely on somebody else to decipher that ;)

pvvx commented 3 years ago

http://wiki.telink-semi.cn/wiki/chip-series/TLSR825x-Series/ BLE Generic TLSR8258 User Guide SDK V4.0.0 Development Handbook http://wiki.telink-semi.cn/doc/an/AN_20050601-E_Telink%20Kite%20Multiple%20Connection%20BLE%20SDK%20Developer%20Handbook.pdf

http://wiki.telink-semi.cn/tools_and_sdk/Driver/doc/kite/html/index.html

pvvx commented 3 years ago

There are problems with the SHTC3 sensor. The solution is described here https://github.com/atc1441/ATC_MiThermometer/issues/134

instead of wake-up via GPIO, setting a wakeup event by timer is used

bls_pm_registerAppWakeupLowPowerCb(WakeupLowPowerCb);
bls_pm_setAppWakeupLowPower(clock_time() + x * CLOCK_16M_SYS_TIMER_CLK_1MS, 1);
pvvx commented 3 years ago

https://pvvx.github.io/MHO_C401/power_original.html

pvvx commented 3 years ago

image image

michapr commented 3 years ago

@pvvx I have updated the C401 some time ago with your code - it is working fine. additional have added an EPD update only when value was changed - this save much energy.

For other room have added the option to change display only if temp. change >0.1 degree and hum. change >1% - also a possible option while we cannot save energy while updating ;)

pvvx commented 3 years ago

The original has a minimum consumption of 52 μA. Alternative firmware under the same conditions - 23 μA. Less makes no sense...