Halting CPU by the "read()" function at the end of the "set_gain(byte gain)" when no response from the module

[HamidSaffari]

Hi and best regards to the creators.
the "read()" function at the end of the "set_gain(byte gain)" cause halting CPU when there is no response from module when calling "begin()" .
I think the simplest solution is to remove the "read()" request at the end of "set_gain(byte gain)" function.

[amotl]

Dear Hamid,

thanks for writing in and for spotting this issue. I will split my answer into two halves.

Does set_gain() really need to call read()?

I think the simplest solution is to remove the "read()" request at the end of "set_gain(byte gain)" function.

That set_gain() needs to access the hardware was exactly the reason to improve the recent library cleanup (#123) in order that the library only starts accessing the hardware with HX711::begin() where set_gain() is called in the end.

Personally, I took it for granted that the library must touch the hardware in order to fulfill the set_gain() command. Maybe @bogde might drop some words about this.

Prevent sketch lockup when no hardware is connected

cause halting CPU when there is no response from module when calling "begin()" .

I was about to point you to the new HX711_timeout_example.ino, but I do recognize now that this is not sufficient as it will also call scale.begin(LOADCELL_DOUT_PIN, LOADCELL_SCK_PIN); first, going down to set_gain() and read(), essentially still getting into the spinlock at the beginning of the read() method, which has been recognized by #76 and #111 already and which I have been trying to converge into #125, as all of this addresses the very same issue from my perspective.

So, while making a humbly start by incorporating @thomasfredericks updates from #96 into #123, I recognize we are not there yet.

With kind regards,
Andreas.

[bogde]

According to the datasheet:

Input and gain selection is controlled by the
number of the input PD_SCK pulses (Table 3).
PD_SCK clock pulses should not be less than 25
or more than 27 within one conversion period, to
avoid causing serial communication error.

So to set gain and channel you need to pulse the clock pin a number of times (25 times for channel A, gain 128, 26 times for channel B, gain 32, 27 times for channel A, gain 64).

However, since that's done when you call the read function anyway, the call to read() in set_gain() can be removed. At least that's what I think, any other feedback is welcome, as usual.

I honestly don't remember why it was done like that, I probably thought the chip needs to have the gain set before the actual reading, but now I don't think that's the case.

[HamidSaffari]

like what bodge said the gain value always sending by any read() requests so I do not see the necessity of read() when changing gain.
but if there's any reason then you can use your new ready-timeout request function like this at the end of set_gain(byte gain) and move the read() inside this condition:
if( wait_ready_timeout(1000) ){
read(); }

[amotl]

Removing digitalWrite() and read() from set_gain()

However, since that's done when you call the read function anyway, the call to read() in set_gain() can be removed.

So, if

HX711/src/HX711.cpp

Lines 98 to 99 in 0b07b31

	digitalWrite(PD_SCK, LOW);
	read();

is actually redundant to

HX711/src/HX711.cpp

Lines 147 to 157 in 0b07b31

	// Set the channel and the gain factor for the next reading using the clock pin.
	for (unsigned int i = 0; i < GAIN; i++) {
	digitalWrite(PD_SCK, HIGH);
	#if ARCH_ESPRESSIF
	delayMicroseconds(1);
	#endif
	digitalWrite(PD_SCK, LOW);
	#if ARCH_ESPRESSIF
	delayMicroseconds(1);
	#endif
	}

then we should just get rid of it and start testing it on real hardware, right? Would you be able to do that, Hamid?

It would be a huge improvement to remove the interaction with the hardware from set_gain() altogether, as it would significantly reduce the chance to get caught into the spin lock when starting the driver before the timeout-enabled functions and will enable the library to operate without any hardware attached at all.

Using wait_ready_timeout()

But if there's any reason then you can use your new ready-timeout request function [...] and move the read() inside the wait_ready_timeout() condition.

While this is the case already in the example code like

HX711/examples/HX711_timeout_example/HX711_timeout_example.ino

Lines 16 to 22 in 0b07b31

	if (scale.wait_ready_timeout(1000)) {
	long reading = scale.read();
	Serial.print("HX711 reading: ");
	Serial.println(reading);
	} else {
	Serial.println("HX711 not found.");
	}

your upcoming patch will finally make it work as intended as it won't block anymore when calling set_gain() before and the hardware is faulty or not even connected at all.

[HamidSaffari]

Yes Andreas but you can not use " if (scale.wait_ready_timeout(1000)) { begin() } " because you should first initialized the pins which happens in begin() function.
and yes I tested all I have said with real hardware. so removing those two lines makes everything happy.

[amotl]

you can not use " if (scale.wait_ready_timeout(1000)) { begin() } " because you should first initialized the pins which happens in begin() function.

Sure, but the overall program will start working as intended as is, because your proposal will mitigate the last chance to preliminary get into the spin lock through calling scale.begin() or scale.set_gain(), right? In other words: Both methods don't have to become timeout-aware at all. I am considering this a good thing, so thanks!

Saying this, I still might be missing an important detail you might want to tell me here. If this is the case, please keep going telling me the truth! Otherwise: All good and go ahead.

yes I tested all I have said with real hardware. so removing those two lines makes everything happy.

Thanks already! Will you be able to send a pull request with your amendments and tell us about the hardware you tested this on?

[S0004152598]

Are you planning to modify your code to avoid this issue?
Thank you!
:)