endail / hx711-pico-c

Implementation of single and multiple HX711 use via RP2040's state machine
https://endail.github.io/hx711-pico-c/
MIT License
32 stars 7 forks source link

hx711_get_value_timeout suggestions #34

Closed hubiscode closed 2 years ago

hubiscode commented 2 years ago

The hx711_get_value_timeout() function takes a pointer to the timeout value as an argument. I think it would be more convenient to pass by value, so you could write

bool ok = hx711_get_value_timeout(&hx, 2500, &val);

I think it might also be good to change the loop to

do {

} while (!timeReached(endTime));

This would allow calling the function with a timeout of zero and still get a value back if one is available. Alternatively, a function

bool hx711_get_value_noblock(hx711_t* const hx, int32_t* const val);

would be good and avoid the overhead of computing the timeout.

BTW, your library is great, I really appreciate you making it available on github.

endail commented 2 years ago

The timeout type was an oversight on my part. I wanted to keep it to microsecond resolution and so decided to use a 64 bit int. But there's really no need for that. The smallest delay I measured between each value from the HX711 being read was around 12 milliseconds. So a 32 bit uint by value should be perfectly fine. I'll change that and give you an update when it's done.

As for the second suggestion, hx711_get_value_timeout is designed to obtain a value within the timeout period. A timeout of zero - to me - indicates a timeout which has already been met and hence the function should stop and use its bool return value to signal a timeout. If you always want to get a value irrespective of time, you can use hx711_get_value.

Doing this should also have no performance hit because the PIO program which acquires values executes independently.

int32_t val;
uint64_t timeout = 0;

if(!hx711_get_value_timeout(&hx, &timeout, &val)) {
    val = hx711_get_value(&hx);
}

But in any case, I can't think of a reason to have a timeout of zero anyway.

Thanks! You might also be interested in pico-scale too.

hubiscode commented 2 years ago

Thank you for the changes.

I am using the hx711 for a sensor in a motor control application, so I want to check if a new value is available as quickly as possible and do other things if there is not. That's why a non-blocking function with zero timeout would be nice for what I'm doing. I understand your explanation that the function is designed to provide a value within a specified timeout, and maybe a separate hx711_get_value_noblock() function would be more appropriate semantically for retrieving a value without blocking if one is available. However, checking if there is a value in the fifo before checking the timeout comes at no cost and makes the edge case work.

endail commented 2 years ago

Thanks for your explanation. I agree with a proposed hx711_get_value_noblock() function. How about:

bool hx711_get_value_noblock(hx711_t* hx, int32_t* val)

Which sets val and returns true if a value was available?

hubiscode commented 2 years ago

That would be perfect.

endail commented 2 years ago

Done! Give that a try and let me know how you go.

hubiscode commented 2 years ago

Works perfectly, thank you very much.