olkal/HX711_ADC

Clock wrap not catered for and other issues from a code read through

Closed this issue · 7 comments

In HX711_ADC::startMultiple(unsigned int t) appears the line

if(millis() < startMultipleTimeStamp + startMultipleWaitTime)

which will fail to work as expected if startMultipleTimeStamp + startMultipleWaitTime wraps around the clock. The fix is to work with durations instead of times.

if ( millis() - startMultipleTimeStamp > startMultipleWaitTime)

See https://www.norwegiancreations.com/2018/10/arduino-tutorial-avoiding-the-overflow-issue-when-using-millis-and-micros/
I will create a PR but it may be a few days before I can do that.

Likewise in int HX711_ADC::startMultiple(unsigned int t, bool dotare)
and I suspect a lot of other places! Yes, further down the same functions for a start.

uint8_t HX711_ADC::conversion24bit()
unsigned long data = 0;
if ((data < 0x000000) || (data > 0xFFFFFF))
the first part can never be true since data is unsigned.

data = data ^ 0x800000; // if the 24th bit is '1', change 24th bit to 0
it will also change the 24th bit from '0' to '1'. Is that intended? I think it is in which case the comment is misleading.

data = data << 1;
if (dout)
{
data++;
}
The first line of that can be written data <<= 1
Depending on how smart the compiler is, data |= 1 may be slightly faster than data++.

I would probably have written this as:
data = (data << 1) | (dout == HIGH ? 1 : 0);
I can't see a value specified for HIGH and LOW in the Arduino docs. If specified to be 1 and 0 then this could be reduced to
data = (data << 1) | dout;

A lot of this, the low/high value code, can be compiled out when not needed and the condition removed from the subtractions.
long HX711_ADC::smoothedData()
{
long data = 0;
long L = 0xFFFFFF;
long H = 0x00;
//for (uint8_t r = 0; r < DATA_SET; r++) {
for (uint8_t r = 0; r < (samplesInUse + IGN_HIGH_SAMPLE + IGN_LOW_SAMPLE); r++)
{
#if IGN_LOW_SAMPLE
if (L > dataSampleSet[r]) L = dataSampleSet[r]; // find lowest value
#endif
#if IGN_HIGH_SAMPLE
if (H < dataSampleSet[r]) H = dataSampleSet[r]; // find highest value
#endif
data += dataSampleSet[r];
}
#if IGN_LOW_SAMPLE
data -= L; //remove lowest value
#endif
#if IGN_HIGH_SAMPLE
data -= H; //remove highest value
#endif
return data;
}

smoothedData()
I am surprised that it does not do the >> divBit

olkal commented

Many thanks Malcolm! All your suggestions is now implemented in 1.2.4