Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor DS18B20 Temperature Conversion Time Handling #1823

Open
echavet opened this issue May 26, 2023 · 0 comments
Open

Refactor DS18B20 Temperature Conversion Time Handling #1823

echavet opened this issue May 26, 2023 · 0 comments

Comments

@echavet
Copy link

echavet commented May 26, 2023

Currently, the DS18B20 driver uses a fixed delay of 1 millisecond (board.io.sendOneWireDelay(pin, 1);) after requesting each sensor to perform a temperature conversion. However, the DS18B20 datasheet specifies that a conversion actually takes up to 750 milliseconds.

This misalignment could potentially lead to inaccuracies when reading from multiple DS18B20 sensors simultaneously, particularly if the requested frequency (options.freq) is shorter than the time required for a temperature conversion. Additionally, it's important to note that the implementation of the delayTask function on the Firmata side is not fully functional and relies on the inclusion of a "FirmataScheduler" module, which merely adds processor cycles and does not provide a true delay mechanism.

I propose refactoring the DS18B20 driver code as follows:

readThermometer = () => {
  let devicesToRead;
  let result;

  if (this.addresses) {
    devicesToRead = this.devices.filter(function (device) {
      const address = getAddress(device);
      return this.addresses.includes(address);
    }, this);
  } else {
    devicesToRead = [this.devices[0]];
  }

  devicesToRead.forEach(device => {
    board.io.sendOneWireReset(pin);
    board.io.sendOneWireWrite(pin, device, CONSTANTS.CONVERT_TEMPERATURE_COMMAND);
  });

  // Removed the following line, which appears to be unnecessary
  // board.io.sendOneWireDelay(pin, 1);

  let conversionTime = 750;  // Time needed for temperature conversion according to the DS18B20 datasheet

  readOne = () => {
    let device;

    if (devicesToRead.length === 0) {
      setTimeout(readThermometer, Math.max(0, freq - conversionTime));  // adjust delay to achieve desired freq
      return;
    }

    device = devicesToRead.pop();
    // read from the scratchpad
    board.io.sendOneWireReset(pin);

    board.io.sendOneWireWriteAndRead(pin, device, CONSTANTS.READ_SCRATCHPAD_COMMAND, CONSTANTS.READ_COUNT, (err, data) => {
      if (err) {
        this.emit("error", err);
        return;
      }

      result = (data[1] << 8) | data[0];
      this.emit("data", getAddress(device), result);

      readOne();
    });
  };

  setTimeout(readOne, conversionTime);  // changed the timing to accommodate the actual conversion time
};

With these changes, the DS18B20 driver seems (but should I need review) to correctly handle temperature conversion times, potentially improving the accuracy and reliability when reading from multiple DS18B20 sensors.

Could you review these changes and confirm that they correctly address the issue?

echavet added a commit to echavet/johnny-five that referenced this issue May 26, 2023
Currently, the DS18B20 driver uses a fixed delay of 1 millisecond (board.io.sendOneWireDelay(pin, 1);) after requesting each sensor to perform a temperature conversion. However, the DS18B20 datasheet specifies that a conversion actually takes up to 750 milliseconds.

This misalignment could potentially lead to inaccuracies when reading from multiple DS18B20 sensors simultaneously, particularly if the requested frequency (options.freq) is shorter than the time required for a temperature conversion. Additionally, it's important to note that the implementation of the delayTask function on the Firmata side is not fully functional and relies on the inclusion of a "FirmataScheduler" module, which merely adds processor cycles and does not provide a true delay mechanism.

rwaldron#1823
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant