bogde / HX711

An Arduino library to interface the Avia Semiconductor HX711 24-Bit Analog-to-Digital Converter (ADC) for Weight Scales.
MIT License
890 stars 537 forks source link

Solution to ESP8266 wdt issue and "yield()" problem #73

Open dagnall53 opened 7 years ago

dagnall53 commented 7 years ago

I believe I have a solution to the wdt timer issue and "yield()" problems. I believe that the problems come from the HX711.read function in HX711.cpp. Here, the code does a yield() whilst waiting for a pin to go low (the "is_ready()" function). if the pin never goes low, the code sticks in this endless loop.

My solution is to avoid the yield() function entirely and replace it with delay(1). delay() function allows the ESP to go off and do "wifi things", but appears more well behaved than yield() and has a definite time delay. My while loop then has a simple counter so if the "is_ready()" is not achieved in a sensible time frame, the code will stop the delays and drop out. I have found that if this occurs, for example by not having a HX711 present.. then the scale.read will return -1 which is easily sensed in code and can be used as a HX711 detect: Modified HX711.cpp code example (partial)
....... reading= scale.read(); if (reading !=-1) { Serial.println(" HX711 PRESENT ");}
else {Serial.println(" No HX711 Present"); }

The chances of a real HX711 responding with -1 are vanishingly small.

I have therefore modified my library ' HX711.cpp' file to remove the "yield()" statement in the HX711.read function with a do while function and a counter... thus:

long HX711::read() { // wait for the chip to become ready byte counter=0; while ((counter <= 250) && (!is_ready())) { // changed to remove yield() and test 250 times maximum.. delay(1); counter ++; } unsigned long value = 0; uint8_t data[3] = { 0 }; uint8_t filler = 0x00; // pulse the clock pin 24 times to read the data data[2] = shiftIn(DOUT, PD_SCK, MSBFIRST); data[1] = shiftIn(DOUT, PD_SCK, MSBFIRST); data[0] = shiftIn(DOUT, PD_SCK, MSBFIRST); (....................etc...)

Cheers.

electrokean commented 7 years ago

Well the correct solution is to call yield(). The problem is that under "non-standard" Arduino environments (like the ESP compilers) the following line in HX711.cpp seems to evaluate as true, and thus yield() gets redefined to a null statement - so the WDT never gets "kicked" #if ARDUINO_VERSION <= 106

These lines for defining yield() are probably best removed as support for really old Arduino environments is hardly worth the hassle and just introduces compatibilty problems with the newer non-AVR targets like ESP.

I don't have the Arduino ESP environment setup on a machine currently, so maybe you can confirm my above suspicion. One way would be to use #pragma like this and see if it returns true.

#if ARDUINO_VERSION <= 106
#pragma "test is true"
#endif

But I do agree that a timeout is probably a good idea and calling delay with a loop counter as you've done will work if delay() correctly yields on the various target platforms.

dagnall53 commented 7 years ago

Kean

Thanks, I will try to see what the pragma comes up with..

Adding the loop counter does prevent infinite loops, and I have use the delay(1) before as it seems better behaved than yield(). I think the counter can be quite low when just doing simple reads, but I think it needs to be larger if the average function is used... I have not explored this fully, but did note that with count about 20 a single read was working, whereas the average was not. My code now uses a max count of 250 and seems to work fine for both averages and single reads. This is another good reason to use delay(1), as it links the max cycle time to the counter, whereas yield() would not.

FYI, I have the code now working in my version of the deeBugger 'scope' software and it is working well.

Many thanks, Dagnall Sent from my iPad

On 8 Jul 2017, at 20:30, Kean Maizels notifications@github.com wrote:

Well the correct solution is to call yield(). The problem is that under "non-standard" Arduino environments (like the ESP compilers) the following line in HX711.cpp seems to evaluate as true, and thus yield() gets redefined to a null statement - so the WDT never gets "kicked"

if ARDUINO_VERSION <= 106

These lines for defining yield() are probably best removed as support for really old Arduino environments is hardly worth the hassle and just introduces compatibilty problems with the newer non-AVR targets like ESP.

I don't have the Arduino ESP environment setup on a machine currently, so maybe you can confirm my above suspicion. One way would be to use #pragma like this and see if it returns true.

if ARDUINO_VERSION <= 106

pragma "test is true"

endif

But I do agree that a timeout is probably a good idea and calling delay with a loop counter as you've done will work if delay() correctly yields on the various target platforms.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

electrokean commented 7 years ago

Yes, with a loop count of only 20, it will not always work, especially in a tight loop like average uses. The HX711 by default does 10 samples per second, so could take up to 100ms before having new data ready.

dagnall53 commented 7 years ago

Thanks. I tried the #pragma thing and the arduino version <106 IS triggered by when compiling esp code. Presumably there is another check that could detect if it was compiling for an esp, but I am afraid I do not know what it is..

Cheers Dagnall

Sent from my iPad

On 9 Jul 2017, at 17:07, Kean Maizels notifications@github.com wrote:

Yes, with a loop count of only 20, it will not always work, especially in a tight loop like average uses. The HX711 by default does 10 samples per second, so could take up to 100ms before having new data ready.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

Jotschi commented 6 years ago

I just replaced all yield calls with delay(1) and now my ESP8266 solution is working. Thanks @dagnall53 for the info.

pearson commented 6 years ago

I came here to post about this problem, only to find that it's a known issue. At least it helped brush up on my problem solving skills... :-)

I'm curious... is ARDUINO_VERSION actually defined for Arduino boards? I've done a text search over every Arduino IDE directory, and then the whole PC, and the only reference to ARDUINO_VERSION that was found was in HX711.cpp. Just to double-check, I plugged in my old Arduino Uno and tried to use ARDUINO_VERSION in some code, but the result was error: 'ARDUINO_VERSION' was not declared in this scope. (This was with a clean install of the Arduino IDE, along with the ESP8266 boards.)

If this really is just a problem for the ESP8266 devices, I'd like to suggest enclosing the #if ARDUINO_VERSION <= 106 block with an #ifndef ESP_H block. Since ESP_H is defined by the core code, and before the user's sketch is included, it should be a reliable way to tell if the board is an ESP8266. Something like:

#ifndef ESP_H
    #if ARDUINO_VERSION <= 106
        // "yield" is not implemented as noop in older Arduino Core releases, so let's define it.
        // See also: https://stackoverflow.com/questions/34497758/what-is-the-secret-of-the-arduino-yieldfunction/34498165#34498165
        void yield(void) {};
    #endif
#endif
llanphar commented 6 years ago

@pearson I just went through the same thing as you wrt ARDUINO_VERSION before coming here and came up equally blank.

However, the platform.txt files for all the boards I have installed does contain this in the compiler command line: -DARDUINO={runtime.ide.version} and my code built with the 1.8.5 IDE sees ARDUINO as 10805. Maybe that's the right define to be checking, at least for newer stuff? I haven't researched to see if this has always been the convention.

Paitch commented 6 years ago

I used both suggestions from @dagnall53 and @electrokean.. and it works!!! And I used this sketch that I found in other forum:

_#include

// Scale Settings const int SCALE_DOUT_PIN = D2; const int SCALE_SCK_PIN = D3;

HX711 scale(SCALE_DOUT_PIN, SCALE_SCK_PIN);

void setup() { Serial.begin(115200); scale.set_scale();// <- set here calibration factor!!! scale.tare(); }

void loop() { float weight = scale.get_units(1); Serial.println(String(weight, 2)); }

And it works fine!

sergio-rivas commented 5 years ago

I used both suggestions from @dagnall53 and @electrokean.. and it works!!! And I used this sketch that I found in other forum:

_#include

// Scale Settings const int SCALE_DOUT_PIN = D2; const int SCALE_SCK_PIN = D3;

HX711 scale(SCALE_DOUT_PIN, SCALE_SCK_PIN);

void setup() { Serial.begin(115200); scale.set_scale();// <- set here calibration factor!!! scale.tare(); }

void loop() { float weight = scale.get_units(1); Serial.println(String(weight, 2)); }

And it works fine!

Can you send your modified version of the library? @Paitch

gavingc commented 2 years ago

Hi,

Noting that this issue is still open; as of tag 0.7.5 this is still not resolved in an ideal way. I think a completely non-blocking read option is required but still need to think about a user friendly, and backwards compatible, way to implement; something like a non-blocking flag on the class or HX711::read_non_blocking()?

In any case, for now I wanted to record this here. On ESP32 when HX711::read() is called from a higher priority task (higher than at least one other task and the idle task) then this implementation does not feed the task watchdog timer (because the idle task never gets a schedule slot to run). Even if it did, it would still rob CPU time from other lower priority tasks on the same core. With other tasks of the same priority (e.g. 1), round robin scheduling would at least give all tasks CPU (except perhaps the idle task which would then still trigger task watchdog).

HX711.h (defaults to 0 ms)

void wait_ready(unsigned long delay_ms = 0);

HX711.cpp (all calls to read values come through read() and always call wait_ready())

bool HX711::is_ready() {
    return digitalRead(DOUT) == LOW;
}

long HX711::read() {

    // Wait for the chip to become ready.
    wait_ready();
       ...
}

void HX711::wait_ready(unsigned long delay_ms) {
    // Wait for the chip to become ready.
    // This is a blocking implementation and will
    // halt the sketch until a load cell is connected.
    while (!is_ready()) {
        // Probably will do no harm on AVR but will feed the Watchdog Timer (WDT) on ESP.
        // https://github.com/bogde/HX711/issues/73
        delay(delay_ms);
    }
}

When the DOUT pin is high, this results in a call of delay(0); which does not feed the watchdog timer. With the default tick of 10 ms, even delay(10) will cause a higher priority task to unblock and preempt other tasks at the tick interval, thus starving them of CPU time.