milesburton / Arduino-Temperature-Control-Library

Arduino Temperature Library
https://www.milesburton.com/w/index.php/Dallas_Temperature_Control_Library
973 stars 487 forks source link

Helper for async delay? #218

Closed Andersama closed 2 years ago

Andersama commented 2 years ago

Without messing about with the library itself it seems like the existing blockTillConversionComplete function could be rewritten to be it's own utility function to smooth out the async feature. The current function is setup to be called immediately after the temperature request is made, but a small change can be made so both synchronous and asynchronous users can make use of it.

Here's the edit what I ended up with.

void wait_for_temps(DallasTemperature &sensor, unsigned long start) {
  if (checkForConversion && !parasite) {
    while (!isConversionComplete() && (millis() - start < MAX_CONVERSION_TIMEOUT ))
      yield();
  } else {
    // not clear on parasite power usage, is supposed to function timing wise, I'm assuming this is ok, the existing code appears to wait the whole maximum duration
    // this seems to suggest that parasite power users don't really benefit from making an async call
    unsigned long now = millis();
    unsigned long spent = now - start;
    unsigned long wait_for = millisToWaitForConversion(bitResolution);
    sensor.activateExternalPullup();
    unsigned long diff = (wait_for - spent);
    unsigned long delay_for = diff < MAX_CONVERSION_TIMEOUT ?  diff : MAX_CONVERSION_TIMEOUT;
    delay(delay_for);
    sensor.deactivateExternalPullup();
  }
}

The usage if blockTillConversionComplete were rewritten would be something like:

  sensors.setWaitForConversion(false); // could be in setup or loop
  sensors.requestTemperatures();
  unsigned long request_start = millis();
  // do other tasks while we're waiting

  // wait out the remaining time needed
  sensors.blockTillConversionComplete(request_start);
  // process temperature reading
  float temp = sensors.getTempCByIndex(0);
RobTillaart commented 2 years ago

Thanks for the ide, I think I understand your proposal.

That timestamp could be made inside the library by requestTemperatures() so the user

If I understand correctly your proposal moves the active waiting part from user code into the library code. IMHO there is little advantage as the complexity is not greatly reduced, or new functionality is added.


Note there is an unwanted side effect in your implementation.

Suppose there have passed 2000 millis between request_start = millis(); and blockTillConversionComplete() Then

Andersama commented 2 years ago

Ah, right good catch, was only focused on making the function's delay between the expected 0-750, but technically it's counting down so I should've flipped it the other way around.

Yeah it's not too big a difference so it's not crazy extra useful, I worked out how to do the same without adding it to the library, so I'm sure other people have as well. On my end the timestamp is just an added parameter to the function call, so it just changes the signature in the library a bit.

Andersama commented 2 years ago

The smallest additional modification to the library I can think of is to have requestTemperatures and related functions return their timestamp, that way if blockTillConverisionComplete() is called later. Then the usage becomes:

  sensors.setWaitForConversion(false); // could be in setup or loop
  unsigned long request_start =  sensors.requestTemperatures();
  // do other tasks while we're waiting

  // wait out the remaining time needed
  sensors.blockTillConversionComplete(request_start);
  // process temperature reading
  float temp = sensors.getTempCByIndex(0);

The other choice is of course to store an unsigned long in the class itself.

RobTillaart commented 2 years ago

requestTemperatures() and related functions return their timestamp

That would be a minimal change, Note: it would also return the timestamp in synchronous (blocking) mode, might have its use too.

uint32_t DallasTemperature::requestTemperatures() 
{
  _wire->reset();
  _wire->skip();
  _wire->write(STARTCONVO, parasite);

  // ASYNC mode?
  if (waitForConversion) blockTillConversionComplete(bitResolution);   // <<<<<<  logic is reverted.
  return millis();
}

The other choice is of course to store an unsigned long in the class itself.

Not sure, there are scenarios that need more than one timestamp I expect. imagine two or more sensors on one bus with different resolutions set. Combined in pairs, one might be read often after 95 millis (fast feedback) and the other after 750 millis (e.g. only when precision is needed).

People using the async mode know what they are doing (assumption alert ;) and can handle a timestamp. They probably have more async actions to handle in their code to keep it reactive.

In fact I do not know if and how a sensor reacts if I call requestTemperatures() when it is already converting. Does it continue or restart? Interesting question.

Andersama commented 2 years ago

The issue I found (accidentally) with async mode was that the temperature probe I have and the lcd screen I had connected cannot be used simultaneously because of the apparent power draw, which I only discovered because sending the lcd any commands in between would corrupt the temperature reading.

I actually have a second probe that I've wanted to use. I hadn't considered using one at a lower resolution, that might be nice trick. Have one low precision value compare to a previous high precision value.

I'm not sure what calling requestTemperatures() does if used back to back. I'd assume it'd ignore it?

Edit: Thinking about it, what happens if multiple readings are called back to back? That'd give it away, if you can read multiple times I'd guess requestTemperatures() would have the probe sample again.

RobTillaart commented 2 years ago

I'm not sure what calling requestTemperatures() does if used back to back. I'd assume it'd ignore it? Edit: Thinking about it, what happens if multiple readings are called back to back? That'd give it away, if you can read multiple times I'd guess requestTemperatures() would have the probe sample again.

These questions needs actual testing ..


For power measurement I often use a stick similar to this

https://asset.conrad.com/media10/add/160267/c1/-/en/002107745DS01/datablad-2107745-joy-it-jt-at34-usb-multimeter.pdf

Andersama commented 2 years ago

Just for reference, (at least with the probe I'm working with), it appears a second requestTemperatures() gets ignored and a probe may be read from multiple times.

Here's a rough sketch of what I tested:

  sensors.requestTemperatures();
  unsigned long ts_request = millis();
  delay(100);
  sensors.requestTemperatures();
  //wait properly from the ts_request timestamp
  float temp = sensors.getTempF(); //
  float temp2 = sensors.getTempF(); // temp appears to be == to temp2
  sensors.requestTemperatures();
  unsigned long ts_request = millis();
  delay(100);
  sensors.requestTemperatures();
  //wait properly from the ts_request timestamp
  float temp = sensors.getTempF(); //
  delay(101); // I don't think the probe can handle two requests in flight, but presumably it should appear after this
  float temp2 = sensors.getTempF(); // temp still appears to be == to temp2
Andersama commented 2 years ago

Closing given #222 was merged.