SUCCESS: Multiple / Single DS18B20 temp sensors on a single OneWire bus


#1

EDITED:
This sketch can be setup to run (1 to n) DS18B20 devices on a particle.io device. I used five. It is flexible in number of devices, sample times (90ms-whatever, or as fast as can be done), bit resolution (9-12bits), publishing max times, publishing min times (based on a defined temperature differential).

It is fast and does not depend on any delay functions except those built into the OneWire protocol. If you have 10 devices hooked up, you could get about 80 temperature conversions/second in 9-bit mode. This program has some built-in monitoring of CRC failures, that were used to find the source of the CRC errors. But with the fixed OneWire.cpp file, there are no errors for the Photon.

Through this project we discovered a bug in the particle.io implementation of OneWire. If you include the fixed “OneWire.cpp” file as part of your compile, and you have one of the “fixable devices”, you will get ZERO CRC errors once you setup your experiment to run correctly (hookups, pullup resistor, etc). The OneWire bus is actually extremely robust when not interrupted in the middle of bit transfers. Hopefully, particle.io will have some suggestions to fix this for ALL its devices in the near future. See this thread for more information:

This is my first real attempt at writing and publishing a sketch that can be used by others. Excuse my coding style, formatting style, and commenting style, this is all new to me. Any feedback on better techniques would be appreciated.

If you find this useful, let me know…I am kind of proud of my first real program that interacts with the environment (temp sensors), haha.

See my final code in about Post 17

=================
ORIGINAL POST STARTS HERE

I’ve been working on some code to include DS18B20 devices in my Photon project to read non-critical temperatures of my pool system (water, ambient air, chlorine tank, etc). I’ve struggled over the last couple days to get a snippet of code working successfully that I would then include into my project.

Not being a programmer (I did assembly and a hardware coding language over 20 years ago), when I have problems, I REALLY have problems. In any case, by delving into the code, I finally got something that works the way that I want. I thought I would post it here, since there are a number of other posts where persons have issues bringing up multiple DS18B20 devices on a single OneWire bus.

Essentially, I have extracted/modified the code to control/start/read temperature conversions from the DS18B20. By doing so, I have eliminated the abstraction level that the DS18B20 library provided and directly interface to the DS18B20s via functions of OneWire.

Why would I do this? Well, there are a few issues I had with the DS18B20 library. First, I believe the code for changing the resolution of DS18B20s is wrong and doesn’t work. It’s possible I was not using it in the correct manner. Second, the DS18B20 library seems to rely on DELAYs (up to .75 seconds) for each conversion requested…and each request must be made individually, one for each device on the OneWire. I really don’t like the use of DELAY (even though I realize the OneWire implementation uses it extensively in much shorter time “bursts”). Third, there seems to be quite a bit of overhead associated with the OneWire and the DS18B20 libraries that I don’t need for my project. I am unclear (since I have really never worked with higher level languages) how unneeded/unused portions of libraries are incorporated into the compiled code…at this point I don’t know how much code I will have in my completed project. Fourth, I had no clue what was going on with the libraries and, since it wasn’t working, the only recourse for me is to start taking out things until I got to a bare minimum of functionality that I understand completely.

My son-in-law tells me that this is not how coding is done these days (for modularity, for testing, etc), but I couldn’t help it, haha. So here it is:

Some restrictions:

  1. There is NO support for parasitic power on the OneWire bus
  2. There is NO support for CRC error checking with this implementation although it could be added back. I won’t need it on my project but will put in some minimal checks to use only “valid” data that is received from the DS18B20 devices. I do see an occasional temperature read failure with this basic implementation. Remember, my temperature readings are NOT critical, I’ll just use the next “good one”. This coding example has absolutely NO error checking that I will eventually include in my project.
  3. This implementation assumes that only DS18B20 devices are attached to the OneWire. There is NO support for its variations.
  4. In order to simplify things, all UNIQUE 8-byte DS18B20 Addresses must be extracted from the devices and put directly into the code. I will not be changing out sensors (unless there is a failure), so it wasn’t worth it for my project to search out the addresses of the DS18B20 devices when my implementation is essentially static. I must add, that this seems to have introduced a LOT of stability to my solution. When I was working with the library, I had multiple, multiple failures to detect all devices and addresses on the OneWire bus. In fact, I don’t think I ever was successful to have it recognize all five of my devices (I bought a five-pack on Amazon) at the same time.

There are only 3 functions of the DS18B20 that I needed and have implemented.

  1. Set the conversion resolution of all DS18B20s (9,10, 11, or 12 bit). This is a function that sets the resolution of all devices simultaneously to the same value.
  2. Start a temperature conversion on the DS18B20s. This is a function that starts a temperature conversion on all devices simultaneously.
  3. Read the temperature recorded by any DS18B20. This addresses and extracts the recorded temperature from a single DS18B20.

This code snippet works well for my setup. I have attached five DS18B20s (each with a 3-meter wire wire) to it. This is also how my final solution will be deployed. Not sure how it would apply to those projects which have their sensors spread out over a larger area. In general, I would say it is a good thing to minimize traffic on the OneWire, that is where most problems/issues seem to occur.

I do observe an occasional failure in the temperature reading, maybe one out of every few hundred (edit: i was wrong about this number). Keep in mind that I will be adding some basic “error checking”/”error ignoring” in my final solution that is not shown in this snippet.

In any case, I would like to hear any feedback good or bad in the process I have gone through. Remember, I really am a neophyte when it comes to all this.

#define ONEWIRE_SEARCH 0  // OneWire option: ignore the search code for devices/device addresses
#define ONEWIRE_CRC 0     // OneWire option: ignore the CRC code
#define ONEWIRE_CRC16 0   // OneWire option: ignore 16-bit CRC code (redundant since CRC is eliminated on prior line)

#include "OneWire.h"
#include <tgmath.h>

//ds18b20 resolution is determined by the byte written to it's configuration register
enum DS18B20_RESOLUTION   : uint8_t {
  DS18B20_9BIT  = 0x1f,         //   9 bit   93.75 ms conversion time
  DS18B20_10BIT = 0x3f,         //  10 bit  187.50 ms conversion time
  DS18B20_11BIT = 0x5F,         //  11 bit  375.00 ms conversion time
  DS18B20_12BIT = 0x7F,         //  12 bit  750.00 ms conversion time
};

//if ds18b20 resolution is less than full 12-bit, the low bits of the data should be masked...
enum DS18B20_RES_MASK   :   uint8_t {
  DS18B20_9BIT_MASK  = 0xf8,        
  DS18B20_10BIT_MASK = 0xfc,      
  DS18B20_11BIT_MASK = 0xfe,        
  DS18B20_12BIT_MASK = 0xff,       
};

//ds18b20 conversion time is ALSO determined by the byte written to it's configuration register
enum DS18B20_CONVERSION_TIME   : uint16_t {
  DS18B20_9BIT_TIME  = 100,         //   9 bit   93.75 ms conversion time w/pad
  DS18B20_10BIT_TIME = 200,         //  10 bit  187.50 ms conversion time w/pad
  DS18B20_11BIT_TIME = 400,         //  11 bit  375.00 ms conversion time w/pad
  DS18B20_12BIT_TIME = 800,         //  12 bit  750.00 ms conversion time w/pad
};

#define DS18B20_PIN_ONEWIRE D2                      //  my system implements OneWire on Photon pin D2
#define NUM_DS18B20_DEVICES 5                       //  my system has FIVE DS18B20 devices attached to OneWire (on pin D2)
#define DS18B20_RESOLUTION DS18B20_11BIT            //  my system uses 11-bit: select appropriate enumerated selection above
#define DS18B20_RES_MASK DS18B20_11BIT_MASK         //  my system uses 11-bit: select appropriate enumerated selection above
#define DS18B20_CONVERSION_TIME DS18B20_11BIT_TIME  //  my system uses 11-bit: select appropriate enumerated selection above

#define DS18B20_SAMPLE_INTERVAL  500        //  defines the DS18B20 Sampling Interval, which determines how often to sample the 
                                            //  DS18B20 devices...this interval rescheduled automatically
                                            // .....should be greater than: DS18B20_CONVERSION_TIME * 5 (# of Sensors) + pad(tbd)
                                            // .....but an "interval check" in the rescheduling should handle a time "violation"

//Publishing Definitions and variables
#define PUBLISH_MAX_INTERVAL       60000        // every 10 seconds  ...(these values change continuously as I am testing my system...this is 60 seconds)
#define PUBLISH_MIN_INTERVAL       1000         // every 1 second 
unsigned long currentMillis;
unsigned long prior_publish_time = 0;           // initialize to a value out in time, will be re-initialized later, TODO: need a better way to do this
bool publishNOW = false;                        // a particular function may request an immediate status publish by making this "true"


// OneWire DS18B20 8-byte unique addresses that must be obtained and entered into the table below
// Use the OneWire example code called Address_Scanner to gather these...
// ...then input them into the array below
//
// ********* REPLACE THESE ADDRESSES WITH THE UNIQUE ADDRESSES OF YOUR DS18B20 DEVICES **************
//
const uint8_t DS18B20_OneWire_ADDRESSES[NUM_DS18B20_DEVICES][8] = 
    {0x28, 0xAA, 0x8D, 0x68, 0x3F, 0x14, 0x01, 0x2E,    // address of first DS18B20 device
     0x28, 0xAA, 0x49, 0x88, 0x3F, 0x14, 0x01, 0x5A,    // address of 2nd DS18B20 device
     0x28, 0xAA, 0x49, 0x67, 0x3F, 0x14, 0x01, 0x89,    // ..
     0x28, 0xAA, 0xB3, 0x6E, 0x3F, 0x14, 0x01, 0x01,    // ..
     0x28, 0xAA, 0xF6, 0x6F, 0x3C, 0x14, 0x01, 0x51};   // address of last DS18B20 device


unsigned long prior_DS18B20_conversion_start = 0, prior_DS18B20_interval_start = 0, current_DS18B20_interval_start = 0;
uint16_t current_temps_raw[NUM_DS18B20_DEVICES] = {70,70,70,70,70};   // current raw readings of temperature sensors
float f_current_temps[NUM_DS18B20_DEVICES];        // current temperature readings from sensors
float f_current_temps_pub[NUM_DS18B20_DEVICES];    // last published temperatures of the sensor 

// temporary (probably) variables for testing and characterizing: error counts and total conversions counter
unsigned long error_count_00 = 0, error_count_ff = 0, error_count_20, error_count_OR, error_count_5;
unsigned long conversion_count = 0;

bool DS18B20_read_conversions = false;
uint8_t DS18B20_ptr = 0;



// Function declarations
void start_DS18B20_Conversions();   
void set_DS18B20_Resolutions(uint8_t resolution);
int16_t read_DS18B20_Conversion(const uint8_t addr[8]);
void doTemperatureCalculations();
bool DS18B20_SamplingComplete();


OneWire ds18b20_onewire(DS18B20_PIN_ONEWIRE);   // instantiate the OneWire bus


void setup() {
  set_DS18B20_Resolutions(DS18B20_RESOLUTION);   //Set resolution of ALL DS18B20s attached to OneWire to the same value and at the same time
}


void loop() 
{
  currentMillis = millis();

  // Publish the status if conditions are met
  if (((currentMillis - prior_publish_time >= PUBLISH_MIN_INTERVAL) && publishNOW) ||
      (currentMillis - prior_publish_time >= PUBLISH_MAX_INTERVAL)) {

    if (publishAllStatus()) {     // function attempts to publish the status
      publishNOW = false;         // ...if successful then get ready for next publish
      for (uint8_t i = 0; i < NUM_DS18B20_DEVICES; i++) {
        f_current_temps_pub[i] = f_current_temps[i];
        publishNOW = false;
        }
    prior_publish_time = currentMillis;                    // setup for the next publish time
    }
  }
  if (DS18B20_SamplingComplete()) doTemperatureCalculations();
}


// The sampling code for the DS18B20 sensors simply starts a conversion on all devices, and then reads the result from each device
// Only one sample is taken from each..I want to get a feeling for IF averaging of successive readings is necessary, etc.
bool DS18B20_SamplingComplete() {
  // Enter this code body if within a valid DS18B20 sampling interval window AND prior DS18B20 temperature conversions have haad time to complete
  if (((currentMillis - prior_DS18B20_conversion_start) >= DS18B20_CONVERSION_TIME)  && 
      ((currentMillis - prior_DS18B20_interval_start) >= DS18B20_SAMPLE_INTERVAL)) {

        // 1) start DS18B20 temperature conversions 2) read one sampled DS18B20 conversion one by one  3) sampling for the interval is complete
    if (!DS18B20_read_conversions && (DS18B20_ptr == 0)) {      
        // starts temperature conversions on all DS18B20 devices attached to the OneWire bus           
      start_DS18B20_Conversions(); 
      prior_DS18B20_conversion_start = millis();                         // capture the start conversion time for determination of finish time
      current_DS18B20_interval_start = prior_DS18B20_conversion_start;   // capture the start time to schedule next interval
      DS18B20_read_conversions = true;
    }
    else if (DS18B20_read_conversions) {
      // no rush, read only ONE of the DS18B20 results. Cycling through them one at a time avoids all OneWire bus traffic within one loop() execution
      current_temps_raw[DS18B20_ptr] = read_DS18B20_Conversion(DS18B20_OneWire_ADDRESSES[DS18B20_ptr]);  
      if (++DS18B20_ptr >= NUM_DS18B20_DEVICES) {  //  advance pointer to next analog channel of ADS1115, check if all DS18B20s have been sampled
        DS18B20_read_conversions = false;
      }
    }
    else {   // here, all samples have been completed, so setup for the next DS18B20 sample interval and return "true" for compeltion
      DS18B20_ptr = 0;         //  ...reset to 0 if 4+, there are only four channels on the DS18B20, AD0:AD3
      prior_DS18B20_interval_start =    // just in case the sample interval was extended or held up (for some reason) and exceeded the DS18B20_SAMPLE_INTERVAL 
        ((currentMillis - DS18B20_SAMPLE_INTERVAL) > current_DS18B20_interval_start) ? currentMillis : current_DS18B20_interval_start;
      return(true);
    }
  }
  return(false);
}



// right now this routine does calculate the temps...but 
// most of this is temporary code to see what the error rate is and what kind of errors I get when reading from the OneBus
// Eventually, there will be some kind of algorithm in here to determine if any given sensor reading is valid or not.   There will also 
// possibly be some error logging to see if in my final "real world" deployment at my pool equipment, is significantly
// different from the bench tests.
void doTemperatureCalculations() {
  float temperature;
  for (uint8_t i = 0; i < NUM_DS18B20_DEVICES; i++) {
    conversion_count++;           //keep track of # of conversions

    if (current_temps_raw[i] > 2048) error_count_ff++;   // reading all 1's check (kinda)
    else if ((current_temps_raw[i] == 0) && !(f_current_temps_pub == 0)) error_count_00++; // reading ALL 0's check
    else if ((current_temps_raw[i] > 880 /* corresponds to 55 celsius, 130 farenheit */) || (current_temps_raw[i] < 1 /* 0.1 farenheit */)) error_count_OR++; //outside range check
    else {
      temperature = current_temps_raw[i] / 16.0 * 1.8 + 32;  // this is the Farenheit calculation read from the ds18b20
      //temperature = current_temps_raw[i] / 16.0;  // this is the Celsius calculation read from the ds18b20
      if (fabs(f_current_temps_pub[i] - temperature) > 5) { // check if current reading is within 5 degrees of last reading
        error_count_5++; // probably an error occurred
        publishNOW = true;
      }
      else if (fabs(f_current_temps_pub[i] - temperature) > 1) publishNOW = true;  // force a publish if temperature has changed by more than 1 degree since last published
      f_current_temps[i] = temperature; 
    }
  }
}


// this function sets the resolution for ALL ds18b20s on an instantiated OneWire
void set_DS18B20_Resolutions(uint8_t resolution)  
{
  ds18b20_onewire.reset();            // onewire intialization sequence, to be followed by other commands
  ds18b20_onewire.write(0xcc);        // onewire "SKIP ROM" command, selects ALL ds18b20s on bus
  ds18b20_onewire.write(0x4e);        // onewire "WRITE SCRATCHPAD" command (requires write to 3 registers: 2 hi-lo regs, 1 config reg)
  ds18b20_onewire.write(100);         // 1) write dummy value (100) to temp hi register
  ds18b20_onewire.write(0);           // 2) write dummy value (0)to temp lo register
  ds18b20_onewire.write(resolution);  // 3) write the selected resolution to configuration registers of all ds18b20s on the bus
}

// this function intitalizes simultaneous temperature conversions for ALL ds18b20s on an instantiated OneWire
void start_DS18B20_Conversions()    
{
  ds18b20_onewire.reset();          // onewire intitialization sequence, to be followed by other commands
  ds18b20_onewire.write(0xcc);      // onewire "SKIP ROM" command, addresses ALL ds18b20s on bus
  ds18b20_onewire.write(0x44);      // onewire wire "CONVERT T" command, starts temperature conversion on ALL ds18b20s
}

// this function returns the RAW temperature conversion result of a SINGLE selected DS18B20 device (via it's address)
int16_t read_DS18B20_Conversion(const uint8_t addr[8])
{
  byte  datal, datah;

  ds18b20_onewire.reset();         // onewire intitialization sequence, to be followed by other commands
  ds18b20_onewire.select(addr);    // issues onewire "MATCH ROM" address which selects a SPECIFIC (only one) ds18b20 device
  ds18b20_onewire.write(0xBE);     // onewire "READ SCRATCHPAD" command, to access selected ds18b20's scratchpad
    // reading TWO bytes (of 9 available) of the selected ds18b20's scratchpad which contain the temperature conversion value
  datal = ds18b20_onewire.read() & DS18B20_RES_MASK;  // low byte should be masked by the resolution of the conversion
  datah = ds18b20_onewire.read();  
  ds18b20_onewire.reset();         // ds18b20 requirement: since only a subeset of the scratchpad is read, a OneWire reset MUST be issued

  return (int16_t) ((datah << 8) | datal);
}


 
// Publishes the status, in my case: specifically sends it to a Google spreadsheet and the PoolController Android app
bool publishAllStatus() {
    return publishNonBlocking(
        "temp",                                         // identifies the specific Google spreadsheet SHEET to record this data
        "{\"T0\":"  + String(f_current_temps[0], 1) +    // "Header name" of identified Google SHEET column where this data is recorded
        ",\"T1\":"  + String(f_current_temps[1], 1) +   
        ",\"T2\":"   + String(f_current_temps[2], 1) +
        ",\"T3\":"   + String(f_current_temps[3], 1) +
        ",\"T4\":"   + String(f_current_temps[4], 1) +
        ",\"CC\":"   + String(conversion_count) +
        ",\"Eff\":"   + String(error_count_ff) +
        ",\"E00\":"   + String(error_count_00) +
        ",\"EOR\":"   + String(error_count_OR) +
        ",\"E5\":"    + String(error_count_5) +
        "}");
}



// A wrapper around Partical.publish() to check connection first to prevent
// blocking. The prefix "pool-" is added to all names to make subscription easy.
bool publishNonBlocking(String name, String message) {
    // TODO replace with a failure queue?
    if (Particle.connected()) {
        bool success = Particle.publish("pool-" + name, message, PUBLIC); // TODO, need to understand ramifications of making this PRIVATE
//        Serial.printlnf("Published \"%s\" : \"%s\" with success=%d",
//                        name.c_str(), message.c_str(), success);
        return success;
    } else {
//        Serial.printlnf("Published \"%s\" : \"%s\" with success=0 no internet",
//                        name.c_str(), message.c_str());
    }
    return false;
}


For latest sample output…see a few posts down


DS18B20 - latest include files - cleanest way to implement with Argon / Xenon
Unstable onewire
Sensor Conversions using an ADS1115 on the I2C Bus
OneWire Library Bug / Proposed FIX (June 2019) Affects DS18B20 and other OneWire devices
#2

It is always interesting to look at things from a different angle. You’ve obviously put a lot of thought and work into this, and we can all respect that.

I recently developed a circuit with a parasitic implementation of this sensor and using a library. I’ve been monitoring that for a little over a month now. While it works, the ratio of “CRC errors” to “successful reads” is not in my comfort zone. I plan is to go back and test the sensor with a 5v power source after I finish my current project. When I do that, I will also see if I can leverage your approach.

Thanks for posting your comments and your code.


#3

Bear, thanks. I have a problem…I just posted this code yesterday and am already making changes to it, haha.

One problem for me is that in the final code for my pool controller, I KNOW that I will have a routine that needs to execute once a second (to keep my pump running at any given rpm). I have no clue how long the user thread of my code will take to execute, and so am trying to minimize things along the way. So for the code I posted, it’s obvious to me now that I should separate (in time) the five temperature sensor reads so that the reads take place on different runs through the loop() code. The bit twiddling that occurs in the OneWire protocol takes a significant amount of time and I think it would not be a good thing to pile that many reads on the OneWire bus into one run through the user loop(). But all this is pretty new to me so I am not positive my thinking is right on that.

Parasitic powering of the OneWire bus seems to introduce a lot of issues…are people using it because they already have existing wires (only 2-conductor)? Otherwise, I am not sure what the appeal is to save a wire…In any case, you are probably well aware that you would probably not be able to issue a “Start Conversion” command to all DS18B20 sensors on a OneWire bus using parasitic power, the simultaneous conversion processes would almost certainly draw more power than could be supplied.

Finally, after I had worked on this a couple days, I found (and more importantly, could understand) this library: DS18B20MINIMAL

I think it is good, especially for those that want a small footprint. It has the correct implementation for setting the resolution, it doesn’t have the delays in the start-conversion/read function, it has all the basic DS18B20 functionality implemented in a minimal way (including CRC) . I wish I had started with that library instead of what I did. If you decide to change your methodology, I recommend you look at, and then start with that.


#4

The first thing that comes to mind is to use a software timer to execute the code once per second. That routine will then execute in the timer thread … independent of loop().


#5

I am familiar with interrupts and had read through that section of the Photon datasheet, I thought I would tackle that if/when I needed to. But software timers…I didn’t know about those and it looks like they directly apply to make my pump task much easier, with much less worry about timing on all the other tasks… Thanks!


#6

It is also a good idea to read sensors in separate loop cycles. Loop() shares a thread with other system tasks, so long loop cycles can really bog the system down.

Thanks for the tip about DS18B20MINIMAL. I’ll add that to my list of things to review. I do like the weather proof DS18B20 One-Wire sensor for outdoor use. I am currently reviewing/testing I2C sensors sensors for indoor use.


#7

So I have changed/updated my code because I am trying to figure out what kind of error checking I need since I am not doing the CRC checks on the OneWire bus. This experiment is not definitive but it gives a good start.

I sampled 5 different temperature “items” for about two hours (using five DS18B20s attached to the Photon’s OneWire implementation) and filtered/graphed the output received directly from the sensors. No averaging was done, only a few checks that threw out some data points entirely and flagged some suspicious entries. Other than the ones that were thrown out, all temperature conversions from the DS18B20s are graphed.

As I was setting up this experiment, I noticed that a majority of errors were because a read of the DS18B20 temperature conversion resulted in essentially reading all 1’s (0xffff). I would guess this error is when the Photon sends out an address for a particular DS18B20, something is corrupted and the device doesn’t recognize the request (and so doesn’t respond) which leaves the OneWire bus at a high state (reading 0xffffh). I didn’t graph those but kept track of the number of errors.

I also noticed that there were occasions where all 0’s were read…I have no explanation but I saw this more when setting up the experiment vs when I ran the actual experiment. The check for all 0’s did result in a burst of zeros read for some conversions that appeared over just a very short period of time…and then disappeared again. I didn’t graph those but kept track of the number of errors.

I also did a rudimentary range check from 0.1 Farenheit to 130 Farenheit (it gets hot here in Phoenix). I didn’t graph those temperature reading outside of that range, but did keep track of the number of errors.

Finally, I did a further rudimentary check where I flagged temperatures whose differential was more than 5 degree different from the previous reading. This was kind of unncessary because I still graphed these points AND as you will see, there are some temperature readings that are obviously in error but they are less than 5 degrees differential from the previous temperature. NOTE: there are a few movements in the graph that are explained because the sensors were physically moved.

Here are the results over the period:

Temperature samples converted: 74,410
Errors Read as all 1s, not graphed: 2795
Errors Read as all 0’s, not graphed: 95
Errors Out of Range, not graphed: 7 (I did not keep these values)
Errors plotted (stars on graph): 8

I know this is a “lab setup” so things will change when I deploy this, but not significantly in my case.

So…I WILL need to implement some error checking, but it looks it can be fairly rudimentary…A large majority of the errors in reading the temperature results look to be easily eliminated (all 1’s, all 0’s). In fact, out of the 74410 samples, only 15 total seemed to have been read incorrectly and/or corrupted AS they were read from the sensors.


#8

That’s very interesting … I live in Minnesota, on the North shore of Lake Superior, where the temperature extremes would be in the -30F to +100F range, so your testing appears to be incomplete :slight_smile:

I’m also curious to know more about those erroneous values and I did some research to see what was involved to perform the CRC check. It doesn’t look all that bad. Here’s an article with some sample code: link

The article is amazingly close to what I found when I researched the code in the library that I am using. Those details are below:



//From DS18B20 library -------------------------------------------------
//	VARIABLES: --------------------------------------------
	  byte         _data[12];
	  byte         _addr[8];
	  byte         _dataCRC;
	  byte         _readCRC;

//	INITIAL SETTINGS: -------------------------------------
	  memset(_data, 0, sizeof(_data));
	  memset(_addr, 0, sizeof(_addr));
	  _dataCRC    = 0; 
	  _readCRC    = 0;

//	READ, CHECK CRC, ERROR RETURN -------------------------
	  for (int i = 0; i < 9; i++)
	  {           // we need 9 bytes
		_data[i] = ds->read();
	  }
	  _dataCRC = (OneWire::crc8(_data, 8)); // calls the ONEWIRE library
	  _readCRC = (_data[8]);
	  if (_dataCRC != _readCRC) return NAN;

//From ONEWIRE library -------------------------------------------------
//	CRC COMPUTATION ---------------------------------------
	uint8_t OneWire::crc8( uint8_t *addr, uint8_t len)
	{
		uint8_t crc = 0;

		while (len--) {
			uint8_t inbyte = *addr++;
			for (uint8_t i = 8; i; i--) {
				uint8_t mix = (crc ^ inbyte) & 0x01;
				crc >>= 1;
				if (mix) crc ^= 0x8C;
					inbyte >>= 1;
			}
		}
		return crc;
        }

#9

There is at least one error in my comments, I actually am only checking from “32.1” Farenheit, not “0.1” Farenheit. So, yes…I am missing a few temperature zones, haha. We almost got down to freezing this year here in the Phoenix area, I don’t think we quite made it.

No, the CRC code isn’t bad…and probably the right thing to do would be to include it. However, I am not over my assembly code programming days and waste way too much time on these details when I should be looking at the bigger picture…but its part of the fun.

I do wonder how many additional errors are caused by the CRC reads themselves, because you have to read 4x the bytes from the DS18B20s. Statistically, it WILL cause more errors, but the good thing is that when you read a conversion without a CRC error, you know you’ve got the value correct.

Anyway, I am already doing more experiments and for now I am going put in some rudimentary error checking and see what the results look like. My basic strategy is this:

  1. I am throwing out the resolution stuff, the data has made me realize that 12-bit is the way to go. When the temperatures are read without an error, they are very consistent. I am not going to do any averaging or multi-sampling (unless there is an error). The good readings ARE what they ARE, and they are accurate. And for most situations (especially my situation), there probably is no need to get temperature readings faster than one per second which is what the DS18B20 seems to be designed for as its default.

  2. For error checking…if a result is different by TBD (.5 Farenheit currently) from the previous conversion, I will automatically re-read the conversion register two more times (I don’t start a new conversion, I just re-read it). At that point, I will choose the 2/3 or the 3/3 number. If there is only one (or zero) “good” reading, I’m just going to throw it out and keep the previous. Another one will come along in ONE second. EDIT: actually, if the 2/3 and 3/3 numbers are all 0’s and/or all 1’s…I am throwing those out, too.

I’m running a version of that code already and the data is looking really good, but I want to make a few changes to monitor/categorize the errors, I want to see exactly what is going on there.


#10

For Reference:
I am using a 4.7kohm resistor as a pullup and these sensors I bought from Amazon:

So here are the results of my experiment today…I think this ends it for me, I am going to move on to another section of my project. This has been pretty interesting. So in the following graph I am using five DS18B20 sensors to sample “different” things. Mostly I am moving them around from a cup of hot water, to a cup of ice water, to room temp, etc.

The numbers are almost too good to believe and I hope I haven’t made a mistake somewhere, but I don’t think I have. Over slightly longer than 2 hours, there were a total of about 28,000 temperature conversions on the five sensors. I used the approach I described in the previous post to detect and “correct” errors:

  1. Conversions that did not deviate by more than .5 degrees Fahrenheit (from the prior conversion) were accepted immediately as valid. Any temperature conversion that deviated by more than .5 degrees from the immediate previously recorded temperature was flagged. In those cases, two more reads of that same conversion register were made to compare to the first read (3 total). Note that new conversions were NOT started, only reads of the already completed conversion.
  2. If two or more notorious all 1’s read were detected in the three samples, that conversion was thrown out and I just kept the prior conversion as the “new” conversion. None of those were detected.
  3. If two of the read s were exactly the same, that value was accepted as valid. If the third byte was mismatched, it was tracked as a “read all 1s error” if it was all 1s. If it just mismatched the other two conversions (and not all 1’s), it was tracked as a “first read bad” error.
  4. If none of the reads matched each other it was tracked as a “2 of 3 bad reads” error. There were only two of those.

There are no range checks. This methodology will result in only one temperature data point never being recorded in the data: temperature resolution (-1) which corresponds to -.0625 celsius or 31.9375 Farenheit.

Here are the results:
image

Here is the graph:

Here is the final code…I did strip out all the change DS18B20 resolution code/comments, I really don’t see any advantage of lowering resolution for 90% of the people. There is no impact to code performance because the conversions happen in parallel to the rest of the code.

#define ONEWIRE_SEARCH 0  // OneWire option: ignore the search code for devices/device addresses
#define ONEWIRE_CRC 0     // OneWire option: ignore the CRC code
#define ONEWIRE_CRC16 0   // OneWire option: ignore 16-bit CRC code (redundant since CRC is eliminated on prior line)

#include "OneWire.h"
#include <tgmath.h>       // Only needed for the fabs() function...thinking about getting rid of this

#define DS18B20_PIN_ONEWIRE D2            //  my system implements OneWire on Photon pin D2
#define NUM_DS18B20_DEVICES 5             //  my system has FIVE DS18B20 devices attached to OneWire (on pin D2)
#define DS18B20_CONVERSION_TIME 800       //  12 bit  750.00 ms conversion time w/pad

#define DS18B20_SAMPLE_INTERVAL  1000       //  defines project specific DS18B20 Sampling Interval, which determines how often to sample the 
                                            //  DS18B20 devices...this interval reschedules automatically
                                            // .....should be greater than: DS18B20_CONVERSION_TIME * NUM_DS18B20_DEVICES + some pad
                                            // .....but an "interval check" in the rescheduling should handle a time "violation"

//Publishing Definitions and variables
#define PUBLISH_MAX_INTERVAL       30000     // every 5 minutes
#define PUBLISH_MIN_INTERVAL       1000      // every 1 second, publishing can occur this fast if a publishNOW is requested by a function
unsigned long prior_publish_time;      
bool publishNOW = false;                     // a particular function may request an immediate status publish by making this "true"
unsigned long currentMillis;                 // set at the beginning of each pass through loop()


// OneWire DS18B20 8-byte unique addresses that must be obtained and entered into the table below
// Use the OneWire example code called Address_Scanner to gather these...
// ...then input them into the array below
//
// ********* REPLACE THESE ADDRESSES WITH THE UNIQUE ADDRESSES OF YOUR DS18B20 DEVICES **************
//
const uint8_t DS18B20_OneWire_ADDRESSES[NUM_DS18B20_DEVICES][8] = 
    {0x28, 0xAA, 0x8D, 0x68, 0x3F, 0x14, 0x01, 0x2E,    // address of first DS18B20 device
     0x28, 0xAA, 0x49, 0x88, 0x3F, 0x14, 0x01, 0x5A,    // address of 2nd DS18B20 device
     0x28, 0xAA, 0x49, 0x67, 0x3F, 0x14, 0x01, 0x89,    // ..
     0x28, 0xAA, 0xB3, 0x6E, 0x3F, 0x14, 0x01, 0x01,    // ..
     0x28, 0xAA, 0xF6, 0x6F, 0x3C, 0x14, 0x01, 0x51};   // address of last DS18B20 device


unsigned long prior_DS18B20_conversion_start = 0, prior_DS18B20_interval_start = 0, current_DS18B20_interval_start = 0;
int16_t current_temps_raw[NUM_DS18B20_DEVICES];    // current raw readings of temperature sensors
float f_current_temps[NUM_DS18B20_DEVICES];        // current temperature readings from sensors
float f_current_temps_pub[NUM_DS18B20_DEVICES];    // last published temperatures of the sensor 


// Function declarations
void start_DS18B20_Conversions();   
int16_t read_DS18B20_Conversion(const uint8_t addr[8]);
void doTemperatureCalculations();
bool DS18B20_SamplingComplete();
bool publishAllStatus();
bool publishNonBlocking(String name, String message);


OneWire ds18b20_onewire(DS18B20_PIN_ONEWIRE);   // instantiate the OneWire bus


void setup() {
  // no need to intialize the DS18B20 devices since I am using the reset defalut of 12-bit conversion resolution
}


void loop() 
{
  currentMillis = millis();

  // Publish the status if conditions are met
  if (((currentMillis - prior_publish_time >= PUBLISH_MIN_INTERVAL) && publishNOW) ||
      (currentMillis - prior_publish_time >= PUBLISH_MAX_INTERVAL)) {

    if (publishAllStatus()) {     // function attempts to publish the status
      publishNOW = false;         // ...if successful then get ready for next publish
      for (uint8_t i = 0; i < NUM_DS18B20_DEVICES; i++) {
        f_current_temps_pub[i] = f_current_temps[i];
        publishNOW = false;
        }
    prior_publish_time = currentMillis;                    // setup for the next publish time
    }
  }
  if (DS18B20_SamplingComplete()) doTemperatureCalculations();
}



// The sampling code for the DS18B20 sensors starts a conversion on all devices, and then reads the result from each device
// Only one sample is taken from each..if the sampled conversion deviates (currently by .5 farenheit) from the prior sample, 
// that same DS18B20 onversion register is then read two more times to make sure it just wasn't a glitch on the OneWire bus.
// Since there is no rush to get these conversions recorded (they happen every second), this function is designed so 
// ...that only one read or write of the OneBus can occur on any given pass through it (I think, haha).  This avoids overloading the 
// ...main loop() with code that takes a long time to execute. 
bool DS18B20_SamplingComplete() {
  static int16_t temperature_read1_raw, temperature_read2_raw, temperature_read3_raw;
  static uint8_t DS18B20_ptr, temperature_read_count;
  static bool DS18B20_conversion_reads_in_progress;

  // Enter this code body if within a valid DS18B20 sampling interval window AND prior DS18B20 temperature conversions have had time to complete
  if (((currentMillis - prior_DS18B20_conversion_start) >= DS18B20_CONVERSION_TIME)  && 
      ((currentMillis - prior_DS18B20_interval_start) >= DS18B20_SAMPLE_INTERVAL)) {

    if (!DS18B20_conversion_reads_in_progress && (DS18B20_ptr == 0)) {      
        // starts temperature conversions on all DS18B20 devices attached to the OneWire bus           
      start_DS18B20_Conversions(); 
      prior_DS18B20_conversion_start = millis();                         // capture the start conversion time for determination of finish time
      current_DS18B20_interval_start = prior_DS18B20_conversion_start;   // capture the start time to schedule next interval
      DS18B20_conversion_reads_in_progress = true;
    }
        // reads one DS18B20 temperature conversion register, if it has changed significantly from its previous value, will "check"
        // ..."checking" occurs by taking 3 samples and comparing them
    else if (DS18B20_conversion_reads_in_progress) {
        // no rush, read only ONE DS18B20 conversion result each time through the code
        // ...cycling through them one at a time avoids multiple OneWire bus traffic delays crammed into one loop() pass
      if (temperature_read_count == 0) temperature_read1_raw = read_DS18B20_Conversion(DS18B20_OneWire_ADDRESSES[DS18B20_ptr]);
      else if (temperature_read_count == 1) temperature_read2_raw =  read_DS18B20_Conversion(DS18B20_OneWire_ADDRESSES[DS18B20_ptr]);
      else temperature_read3_raw =  read_DS18B20_Conversion(DS18B20_OneWire_ADDRESSES[DS18B20_ptr]);

        // if temperature conversion reading has changed >.5 degrees, read the SAME conversion TWO more times to decide which is right
        // '4' corresponds to roughly .5 degrees farenheit, .25 degrees celsius
      if (abs(temperature_read1_raw - current_temps_raw[DS18B20_ptr]) > 4) temperature_read_count++;

        // use all 3 reads of the converted temperature to determine what value should be chosen
        // My experiment showed that most failures are due to reading all 1 bits (most probable) 
        // ...probably a result of the master on the OneWire reading all 1's because a DS18B20 did not recognize its address
        // EXIT of this IF statement will have desired conversion value in "temperature_read1_raw"
      if (temperature_read_count >= 3) {
          // use previous conversion value if more than one read was all 1's, all 1's corresponds to a reading of -1 RAW temp AND
          // is a specific problem with the OneWire bus.   Throw this value conversion out entirely.
        if (((temperature_read1_raw == -1) && (temperature_read2_raw == -1)) ||
            ((temperature_read1_raw == -1) && (temperature_read3_raw == -1)) ||
            ((temperature_read2_raw == -1) && (temperature_read3_raw == -1))) {
          temperature_read1_raw = current_temps_raw[DS18B20_ptr];
        }  
          // temperature has changed, keep the first read (temperature_read1_raw) if it is matched by either the 2nd or 3rd read
        else if ((temperature_read1_raw == temperature_read2_raw) || (temperature_read1_raw == temperature_read3_raw)) {}
          // temperature has changed, but the first reading was wrong, use 2nd value if it matches the 3rd
        else if ((temperature_read2_raw == temperature_read3_raw)) temperature_read1_raw = temperature_read2_raw;
          // all 3 readings of the conversion were different, ignore them and use the previous conversion value
        else temperature_read1_raw = current_temps_raw[DS18B20_ptr];  

        temperature_read_count = 0;
      }

      if (temperature_read_count == 0) {
          // the read of the DS18B20 conversion register has completed, store the value and
          // advance pointer to next DS18B20, and check to see if all DS18B20s conversions have bee read
        current_temps_raw[DS18B20_ptr] = temperature_read1_raw;
        if (++DS18B20_ptr >= NUM_DS18B20_DEVICES) {  
          DS18B20_conversion_reads_in_progress = false;
        }
      }
    }
    else {   // here, all sampled conversion have been recorded, so setup for the next DS18B20 sample interval 
      DS18B20_ptr = 0;         //  ...reset to 0 
      prior_DS18B20_interval_start =    // just in case the sample interval was extended or held up (for some reason) and exceeded the DS18B20_SAMPLE_INTERVAL 
        ((currentMillis - DS18B20_SAMPLE_INTERVAL) > current_DS18B20_interval_start) ? currentMillis : current_DS18B20_interval_start;
      return(true);
    }
  }
  return(false);
}



// temperature calculations and storage based on values read from the DS18B20 conversions
void doTemperatureCalculations() {
  float temperature;
  for (uint8_t i = 0; i < NUM_DS18B20_DEVICES; i++) {
    temperature = current_temps_raw[i] / 16.0 * 1.8 + 32;  // this is the Farenheit calculation read from the ds18b20
    //temperature = current_temps_raw[i] / 16.0;  // this is the Celsius calculation read from the ds18b20
    if (fabs(f_current_temps_pub[i] - temperature) > 1) publishNOW = true;  // force a publish if temperature has changed by more than 1 degree since last published
    f_current_temps[i] = temperature; 
  }
}


// this function intitalizes simultaneous temperature conversions for ALL ds18b20s on an instantiated OneWire
void start_DS18B20_Conversions()    
{
  ds18b20_onewire.reset();          // onewire intitialization sequence, to be followed by other commands
  ds18b20_onewire.write(0xcc);      // onewire "SKIP ROM" command, addresses ALL ds18b20s on bus
  ds18b20_onewire.write(0x44);      // onewire wire "CONVERT T" command, starts temperature conversion on ALL ds18b20s
}

// this function returns the RAW temperature conversion result of a SINGLE selected DS18B20 device (via it's address)
int16_t read_DS18B20_Conversion(const uint8_t addr[8])
{
  byte  datal, datah;

  ds18b20_onewire.reset();         // onewire intitialization sequence, to be followed by other commands
  ds18b20_onewire.select(addr);    // issues onewire "MATCH ROM" address which selects a SPECIFIC (only one) ds18b20 device
  ds18b20_onewire.write(0xBE);     // onewire "READ SCRATCHPAD" command, to access selected ds18b20's scratchpad
    // reading TWO bytes (of 9 available) of the selected ds18b20's scratchpad which contain the temperature conversion value
    //datal = ds18b20_onewire.read() & DS18B20_RES_MASK;  // low byte should be masked by the resolution of the conversion
  datal = ds18b20_onewire.read();  // ...but I am using maximum resolution in my application from now on
  datah = ds18b20_onewire.read();  
  ds18b20_onewire.reset();         // ds18b20 requirement: since only a subeset of the scratchpad is read, a OneWire reset MUST be issued

  return (int16_t) ((datah << 8) | datal);
}


// Publishes the status, in my case: specifically sends it to a Google spreadsheet and the PoolController Android app
bool publishAllStatus() {
    return publishNonBlocking(
        "temp",                                         // identifies the specific Google spreadsheet SHEET to record this data
        "{\"T0\":"  + String(f_current_temps[0], 1) +    // "Header name" of identified Google SHEET column where this data is recorded
        ",\"T1\":"  + String(f_current_temps[1], 1) +   
        ",\"T2\":"   + String(f_current_temps[2], 1) +
        ",\"T3\":"   + String(f_current_temps[3], 1) +
        ",\"T4\":"   + String(f_current_temps[4], 1) +
        "}");
}


// A wrapper around Partical.publish() to check connection first to prevent
// blocking. The prefix "pool-" is added to all names to make subscription easy.
bool publishNonBlocking(String name, String message) {
    // TODO replace with a failure queue?
    if (Particle.connected()) {
        bool success = Particle.publish("pool-" + name, message, PUBLIC); // TODO, need to understand ramifications of making this PRIVATE
//        Serial.printlnf("Published \"%s\" : \"%s\" with success=%d",
//                        name.c_str(), message.c_str(), success);
        return success;
    } else {
//        Serial.printlnf("Published \"%s\" : \"%s\" with success=0 no internet",
//                        name.c_str(), message.c_str());
    }
    return false;
}



#11

Bear, you told me “It is also a good idea to read sensors in separate loop cycles. Loop() shares a thread with other system tasks, so long loop cycles can really bog the system down.” [how do you quote someone with a reply…I don’t see a button to do that as in most forums]

I’ve been thinking about this to determine what exactly you mean…and sorry, I’m still at a loss on how to apply it, haha. I actually made some code changes after you commented (unfortunately my original code is not posted anymore), but after thinking about it, I don’t think it is what you intended to tell me.

For instance: the function in my code called bool “DS18B20_SamplingComplete()”, which actually calls the reads/writes to do the sensor readings.

I “pulled” that entire section of code out of the main loop() and put it in its own function. But it is still called from the main loop() on every cycle. Here is why I am confused. Whether or not it is in the main loop or the separate function, it is still “called” the same number of times. The millis() check “test” at the beginning of that code is done on every cycle through loop().

However, due to those millis() checks, the BODY of that DS18B20_SamplingComplete() code is only executed 7 times per DS18B20_SAMPLE_INTERVAL (1 to start conversions, 1 for each sensor read: 5 in my case, 1 for finish…if there are errors on the read +2 for each occurrence). Currently that sample interval is defined as 1 second…I am reading all sensor temperature conversions once a second…but that could be anything larger than the time required to do the actual conversions (750ms for 12-bit resolution): 1 minute, 1 hour, 1 day.

So, does what I did meet your intention, or are you talking about moving the sensor code to an entirely different loop cycle, similar to the timer function we talked about earlier? And if so, what would that “other” loop cycle be?

Thanks


#12

You mark the text you want to quote with the mouse and then there should appear "Quote" button" like here


#13

System firmware performs communication and other vital tasks between loop() cycles, so a well written application will cede the processor frequently to assure the system can perform those tasks.

Based on comments in the DS18B20 library code, reading the sensor can take hundreds of milliseconds. That would led me to suggest that it would be a good idea to read each sensor in a separate loop cycle with logic like …

time_t nextSensorReadTime = 0;
int sensorReadInterval = 30; // seconds
int sensorCount = 0;

setup() { }

loop() { 
    if (Time.now() >= nextSensorReadTime) {
        if (sensorCount  < 5) {
            read sensor[sensorCount++];
        } else {
            publish sensor data
            sensorCount = 0;
            nextSensorReadTime = nextSensorReadTime + sensorReadInterval;
        }
    }
}

Please forgive the mixture of code and psuedo-code.


#14

The challenge with any OneWire library is that they use bit-banging with delayMicroseconds() for timing and disable interrupts to ensure that FreeRTOS doesn’t preempt use code while OneWire is doing its thing. The problem is that’s really not DeviceOS
friendly. This is one of the main cause of CRC errors and erroneous readings.

If reliability is a key factor, then I suggest using a Maxim DSDS2482-100 I2C-to-1Wire master chip. @rickkas7 wrioe a great library for this device.


#15

Thanks Scruff for the info on how to quote!

Your explanation is exactly what I was thinking…and because of my use of millis(), effectively I have isolated all my sensor code from the main loop() in the fashion that you describe. The code in that sensor section rarely executes as I described in my previous post. In addition, because I am not using the DS18B20 library which includes that delay(750ms) command, the DS18B20 OneWire commands are pretty fast. Starting the conversions takes less than 2ms and reading a conversion takes only about 10ms as compared to the library which integrates both the “start and the read”, but takes 750ms.

Yes, I haven’t seen the errors that others are seeing in this example and I suspect that is because I am not running a bunch of other code along side it. But I have looked at the OneWire code and it seems pretty nice how it is implemented, interrupts are only disabled/enabled during time bit transfer time and the protocol IS designed to transfer a bit at a time.

Its seems to me, based on this experiment, that almost all my errors (which are few) are due to a DS18B20 device failing to recognize that it is supposed to “drive” back a requested conversion data block (of 8 bytes)…resulting in the bus remaining at a high for that entire block “transfer” : results in a CRC error and reading of -1. But I don’t have a logic analyzer, haha.


#16

Im having similar issues with my Xenon, connected over Mesh. The addition of the ATOMIC_BLOCK() helpt the issue, but its not the full picture. at least when a mesh connection is involved. I have added some pin toggles that im watching with a logic analyser, and even inside a ATOMIC_BLOCK(), something is messing with the timing, adding 15-43uS to a few lines of code with just delays and pin toggles. It initially happed on my Argon as well. and then i reconfigured it without being a mesh gateway: and problem gone!


#17

KThis sketch can be setup to run (1 to n) DS18B20 devices on a particle.io device. I used five. It is flexible in number of devices, sample times (90ms-whatever, or as fast as can be done), bit resolution (9-12bits), publishing max times, publishing min times (based on a defined temperature differential).

It is fast and does not depend on any delay functions except those built into the OneWire protocol. If you have 10 devices hooked up, you could get about 80 temperature conversions/second in 9-bit mode. This program has some built-in monitoring of CRC failures, that were used to find the source of the CRC errors. But with the fixed OneWire.cpp file, there are no errors for the Photon.

Through this project we discovered a bug in the particle.io implementation of OneWire. Mentioned by @remcohn in post above. If you include the fixed “OneWire.cpp” file as part of your compile, and you have one of the “fixable devices”, you will get ZERO CRC errors once you setup your experiment to run correctly (hookups, pullup resistor, etc). The OneWire bus is actually extremely robust when not interrupted in the middle of bit transfers. Hopefully, particle.io will have some suggestions to fix this for ALL its devices in the near future. See this thread for more information:

This is my first real attempt at writing and publishing a sketch that can be used by others. Excuse my coding style, formatting style, and commenting style, this is all new to me. Any feedback on better techniques would be appreciated.

If you find this useful, let me know…I am kind of proud of my first real program that interacts with the environment (temp sensors), haha.

Here is the final code…I probably won’t be updating this here…I am trying to figure out GitHub.

/*
 * Project TempSensors
 * Description: A flexible program that has many options to configure any number of DS18B20 devices on a single OneWire bus
 * Author: John Carrieres
 * Date: 6/21/2019
 */

// If ONLY_ONE DS18B20 is being put on the bus AND you don't want to find and input is UNIQUE address, 
// search for "ONLY_ONE" to find the five or six places in code that need to change 
// I haven't actually tested that though...


SYSTEM_THREAD(ENABLED);


#define ONEWIRE_SEARCH 0  // OneWire option: ignore the search code for devices/device addresses
#define ONEWIRE_CRC 1     // OneWire option: enable the CRC code
#define ONEWIRE_CRC16 0   // OneWire option: ignore 16-bit CRC code (redundant since CRC is eliminated on prior line)

#include "OneWire.h"
#include <tgmath.h>       // Only needed for the fabs() function...thinking about getting rid of this


//ds18b20 resolution is determined by the byte written to it's configuration register
enum DS18B20_RESOLUTION   : uint8_t {
  DS18B20_9BIT  = 0x1f,         //   9 bit   93.75 ms conversion time
  DS18B20_10BIT = 0x3f,         //  10 bit  187.50 ms conversion time
  DS18B20_11BIT = 0x5F,         //  11 bit  375.00 ms conversion time
  DS18B20_12BIT = 0x7F,         //  12 bit  750.00 ms conversion time
};

//if ds18b20 resolution is less than full 12-bit, the low bits of the data should be masked...
enum DS18B20_RES_MASK   :   uint8_t {
  DS18B20_9BIT_MASK  = 0xf8,        
  DS18B20_10BIT_MASK = 0xfc,      
  DS18B20_11BIT_MASK = 0xfe,        
  DS18B20_12BIT_MASK = 0xff,       
};

//ds18b20 conversion time is ALSO determined by the byte written to it's configuration register
enum DS18B20_CONVERSION_TIME   : uint16_t {
  DS18B20_9BIT_TIME  = 94,          //   9 bit   93.75 ms conversion time w/pad
  DS18B20_10BIT_TIME = 188,         //  10 bit  187.50 ms conversion time w/pad
  DS18B20_11BIT_TIME = 375,         //  11 bit  375.00 ms conversion time w/pad
  DS18B20_12BIT_TIME = 750,         //  12 bit  750.00 ms conversion time w/pad
};

#define DS18B20_PIN_ONEWIRE D2                       //  my system implements OneWire on Photon pin D2
#define NUM_DS18B20_DEVICES 5                        //  my system has FIVE DS18B20 devices attached to OneWire (on pin D2)
#define DS18B20_CONVERSION_TIME DS18B20_12BIT_TIME   //  match desired enumerated conversion time above
#define DS18B20_RESOLUTION  DS18B20_12BIT            //  match desired enumerated resolution above
#define DS18B20_RES_MASK DS18B20_12BIT_MASK          //  match desired enumerated resolution mask above (low bits at lower resolutions mean nothing)
#define DS18B20_CRC_RETRIES 2                        //  define how many DS18B20 CRC failure retries are done before moving on
#define DS18B20_FAIL_CRC_VALUE 0x07ff                //  returned when a CRC Fail Condition occurs: =2047 decimal...177 degree celsius...way outside of spec 
#define DS18B20_TEMP_HI_REG 0x55                     //  set to a known value, checkerboard pattern (could be used to abort a "going to fail" crc check)
#define DS18B20_TEMP_LO_REG 0xAA                     //  set to a known value, checkerboard pattern (ditto)

#define DS18B20_SAMPLE_INTERVAL  1000       //  defines project specific DS18B20 Sampling Interval, which determines how often to sample the 
                                            //  DS18B20 devices...in this code, interval reschedules automatically, but could be changed or
                                            //          implemented as a one-shot. 
                                            //  .....for periodic sampling, should be set to: DS18B20_CONVERSION_TIME + pad....but it doesn't matter
                                            //  IF SET to 0, temperature conversions are started and re-started as quickly as possible

//Publishing definitions and variables
#define PUBLISH_MAX_INTERVAL       300000     // every 5 minutes
#define PUBLISH_MIN_INTERVAL       1000     // every 1 second, publishing can occur this fast if a function requests a publishNOW, minimum 1000

  // publishing temperature differential, publish the data immediately (subject to PUBLISH_MIN_INTERVAL) if its 
  // temperature differential from the previous PUBLISHED value is greater than this number...used in a floating point comparison
  // ...An easy way to get quick publishes during testing...grab a probe with your hand and raise its temperature
#define PUBLISH_TEMPERATURE_DIFF      1     


bool publishNOW;                            // a particular function may request an immediate publish by setting this true
unsigned long currentMillis;                // set at the beginning of each pass through loop()
long int conversion_count_DS18B20;              //TESTING CODE , keeps track of the total # of DS18B20 conversions
long int crc_error_count_DS18B20;               //TESTING CODE , keeps track of the total # of CRC errors in DS18B20 conversions
long int crc_fail_count_DS18B20;                //TESTING CODE , keeps track of the total # of CRC failures (all tries) in DS18B20 conversions


// OneWire DS18B20 8-byte unique addresses that must be obtained and entered into the table below
// Use the OneWire example code called Address_Scanner to gather these...
// ...then input them into the array below.  
// Trying to to discover the addresses on the fly has proven to be troublesome for many who use multiple DS18B20 devices on a single OneWire
// ...it's more efficient to determine them and save them once forever, unless sensors in your system are constantly being swapped 
// ...for new ones...NOTE: finding addresses might be easy now with the FIX to the OneWire code that has been floated out there
// ONLY_ONE: addresses are not needed if there is ONLY_ONE DS18B20 on a OneWire bus, it is faster to not use them, but you still can use them
// If you don't want to find this unique addess for ONLY_ONE device, see the top of this file regarding ONLY_ONE device.
//
// ********* REPLACE THESE ADDRESSES WITH THE UNIQUE ADDRESSES OF YOUR DS18B20 DEVICES **************
//
const uint8_t DS18B20_OneWire_ADDRESSES[NUM_DS18B20_DEVICES][8] = 
    {0x28, 0xAA, 0x8D, 0x68, 0x3F, 0x14, 0x01, 0x2E,    // address of first DS18B20 device
     0x28, 0xAA, 0x49, 0x88, 0x3F, 0x14, 0x01, 0x5A,    // address of 2nd DS18B20 device
     0x28, 0xAA, 0x49, 0x67, 0x3F, 0x14, 0x01, 0x89,    // ..
     0x28, 0xAA, 0xB3, 0x6E, 0x3F, 0x14, 0x01, 0x01,    // ..
     0x28, 0xAA, 0xF6, 0x6F, 0x3C, 0x14, 0x01, 0x51};   // address of last DS18B20 device


int16_t current_temps_raw[NUM_DS18B20_DEVICES];    // current raw readings from temperature sensors
float f_current_temps[NUM_DS18B20_DEVICES];        // current temperature readings from sensors
float f_current_temps_pub[NUM_DS18B20_DEVICES];    // last published temperatures readings from sensors


// Function declarations
void start_DS18B20_Conversions();   
int16_t read_DS18B20_Conversion(const uint8_t addr[8], uint8_t ptr);
void doTemperatureCalculations();
bool DS18B20_SamplingComplete();
bool publishAllStatus();
bool publishNonBlocking(const char sheet_name, const char message);
void publishData();
bool timeToPublish();


OneWire ds18b20_onewire(DS18B20_PIN_ONEWIRE);   // instantiate the OneWire bus


void setup() {
  set_DS18B20_Resolutions(DS18B20_RESOLUTION); 

}


void loop() 
{
  currentMillis = millis();

  // Publish the status if conditions are met
  if (timeToPublish()) publishData();

  // When ready, update the current DS18B20 temperature readings
  if (DS18B20_SamplingComplete()) doTemperatureCalculations();
}



// function that publishes selected data...this will be expanded
void publishData(){
  if (publishAllStatus()) {     // function attempts to publish the status
    publishNOW = false;         // ...if successful then get ready for next publish
    for (uint8_t i = 0; i < NUM_DS18B20_DEVICES; i++) {
      f_current_temps_pub[i] = f_current_temps[i];  // update the published temperaturre data
      //other stuff to be added here
    }
  }
}

// function to check if it is time to Publish: either forced (publishNOW) or a timeout of the PUBLISH_MAX_INTERVAL
// and then setup for the next publish event
bool timeToPublish() {
  static long prior_publish_time;      
  if (((currentMillis - prior_publish_time >= PUBLISH_MIN_INTERVAL) && publishNOW) ||
      (currentMillis - prior_publish_time >= PUBLISH_MAX_INTERVAL)) {
    prior_publish_time = currentMillis;                    // setup for the next publish time
    //publishNOW = false;

    return(true);
  }
  return(false);
}


// This code starts a conversion on all DS18B20s simultaneously, and then, later when the conversions are finished, reads the results
// There is only one sampled conversion for each DS18B20..if the sampled conversion fails the CRC checks, a previous sampled conversion is kept 
// Since there is no rush to get these conversions recorded, this function is designed so 
// ...that only one conversion read happens on any given pass through it.  This avoids cramming 
// ...a bunch of execution time into one particular pass of the user code.
bool DS18B20_SamplingComplete() {
  static long prior_DS18B20_interval_start = 10000; 
  static long prior_DS18B20_conversion_start = 10000;
  static long current_DS18B20_interval_start = 20000;
  static int16_t temperature_read_raw;
  static uint8_t DS18B20_ptr = 0;
  static bool DS18B20_conversion_reads_in_progress = false;


  // Enter the code body ONLY if within a valid DS18B20 sampling interval window AND prior DS18B20 temperature conversions have had time to complete
  if (((currentMillis - prior_DS18B20_conversion_start) >= DS18B20_CONVERSION_TIME)  && 
      ((currentMillis - prior_DS18B20_interval_start) >= DS18B20_SAMPLE_INTERVAL)) {

    if (!DS18B20_conversion_reads_in_progress && (DS18B20_ptr == 0)) {      
        // starts temperature conversions on all DS18B20 devices attached to the OneWire bus           
      start_DS18B20_Conversions(); 
      prior_DS18B20_conversion_start = millis();                         // capture conversion start so the "reads" can be scheduled
      current_DS18B20_interval_start = prior_DS18B20_conversion_start;   // capture the start time so next interval can be scheduled
      DS18B20_conversion_reads_in_progress = true;
      conversion_count_DS18B20 += NUM_DS18B20_DEVICES; //TESTING: keeps track of the # of temperature conversions since reset
    }
    else if (DS18B20_conversion_reads_in_progress) {
        // reads one of the DS18B20 temperature conversions
      temperature_read_raw = read_DS18B20_Conversion(DS18B20_OneWire_ADDRESSES[DS18B20_ptr], DS18B20_ptr); //if ONLY_ONE DS18B20, take out the address reference
      
      if (temperature_read_raw != DS18B20_FAIL_CRC_VALUE)
        current_temps_raw[DS18B20_ptr] = temperature_read_raw; 
      else crc_fail_count_DS18B20++;  //TESTING else keep the old value, there were CRC failures on the intial read AND retries

      if (++DS18B20_ptr >= NUM_DS18B20_DEVICES)   
        DS18B20_conversion_reads_in_progress = false;  // all DS18B20 conversions have been read
    }
    else {              // all sampled conversion have been recorded, so setup for the next DS18B20 sample interval 
      DS18B20_ptr = 0;                  //  setup to read the sensors again
      prior_DS18B20_interval_start =    // check if (for any reason) it took longer than DS18B20_SAMPLE_INTERVAL to get the conversions
        ((currentMillis - current_DS18B20_interval_start) > DS18B20_SAMPLE_INTERVAL) ? millis() : current_DS18B20_interval_start;
      return(true);
    }
  }
  return(false);
}


// This does the temperature calculations (from the RAW values) and stores them,
// the latest results are updated and always available within these global arrays: 
// RAW values: current_temps_raw[NUM_DS18B20_DEVICES], these are the integer values read from the sensors
// current temperatures:  f_current_temps[NUM_DS18B20_DEVICES]
void doTemperatureCalculations() {
  float temperature;
  for (uint8_t i = 0; i < NUM_DS18B20_DEVICES; i++) {
    //temperature = current_temps_raw[i] / 16.0;  // this is the Celsius calculation read from the ds18b20
    temperature = current_temps_raw[i] / 16.0 * 1.8 + 32;  // this is the Farenheit calculation read from the ds18b20
         // force a publish if temperature has changed by more than 1 degree since last published
    if (fabs(f_current_temps_pub[i] - temperature) > PUBLISH_TEMPERATURE_DIFF) publishNOW = true;  
    f_current_temps[i] = temperature; 
  }
}



// this function sets the resolution for ALL ds18b20s on an instantiated OneWire
void set_DS18B20_Resolutions(uint8_t resolution)  
{
  ds18b20_onewire.reset();        // onewire intialization sequence, to be followed by other commands
  ds18b20_onewire.write(0xcc);    // onewire "SKIP ROM" command, selects ALL ds18b20s on bus
  ds18b20_onewire.write(0x4e);    // onewire "WRITE SCRATCHPAD" command (requires write to 3 registers: 2 hi-lo regs, 1 config reg)
  ds18b20_onewire.write(DS18B20_TEMP_HI_REG);   // 1) write known value to temp hi register 
  ds18b20_onewire.write(DS18B20_TEMP_LO_REG);   // 2) write known value to temp lo register
  ds18b20_onewire.write(resolution);            // 3) write selected resolution to configuration registers of all ds18b20s
}


// this function intitalizes simultaneous temperature conversions for ALL ds18b20s on an instantiated OneWire
void start_DS18B20_Conversions()    
{
  ds18b20_onewire.reset();          // onewire intitialization sequence, to be followed by other commands
  ds18b20_onewire.write(0xcc);      // onewire "SKIP ROM" command, addresses ALL (or one if there is only one) ds18b20s on bus
  ds18b20_onewire.write(0x44);      // onewire wire "CONVERT T" command, starts temperature conversion on ALL ds18b20s
}

/* 
// A fast conversion read routine for a temperature from from a DS18B20 device, no CRC checking and
// avoids having to read all 9 bytes over that slooooooowwww OneWire
int16_t fast_read_DS18B20_Conversion(const uint8_t addr[8])  
{
  byte  data[2];

  ds18b20_onewire.reset();          // onewire intitialization sequence, to be followed by other commands
  ds18b20_onewire.select(addr);     // issues onewire "MATCH ROM" address which selects a SPECIFIC (only one) ds18b20 device
  ds18b20_onewire.write(0xBE);      // onewire "READ SCRATCHPAD" command, to access selected ds18b20's scratchpad
  data[0] = ds18b20_onewire.read(); // low byte of temperature conversion
  data[1] = ds18b20_onewire.read(); // high byte of temperature conversion
  ds18b20_onewire.reset();          // per spec...if not reading all 9 bytes of the scratchpad, a reset must be issued
  return ((int16_t) (data[1] << 8) | (data[0] & DS18B20_RES_MASK));
}
*/

// this function returns the RAW temperature conversion result of a SINGLE selected DS18B20 device (via it's address)
// If there is a CRC failure in the process, the previously converted result is just re-read...a new conversion is not started.
// It is reattempted up to DS18B20_CRC_RETRIES times
// The pointer to a particular DS18B20 was addeed as a parameter for testing purposes  to check if a particular DS18B20 device
// was having issues with the OneWire Protocol.   I'm leaving it for now
int16_t read_DS18B20_Conversion(const uint8_t addr[8], uint8_t ptr)  // if ONLY_ONE DS18B20, take out address reference: read_DS18B20_Conversion(uint8_t ptr)
{
  byte  data[9];
  bool crc_error;
  int crc_retries = 0;

  do {
    ds18b20_onewire.reset();          // onewire intitialization sequence, to be followed by other commands
    ds18b20_onewire.select(addr);     // issues onewire "MATCH ROM" address which selects a SPECIFIC (only one) ds18b20 device
      //if ONLY_ONE DS18B20, replace the line above  "ds18b20_onewire.select(addr);" with the one directly below
      //  ds18b20_onewire.write(0xcc);      // onewire "SKIP ROM" command, selects the ONLY_ONE ds18b20 on bus without needing address
      //
    ds18b20_onewire.write(0xBE);      // onewire "READ SCRATCHPAD" command, to access selected ds18b20's scratchpad
      // reading the bytes (9 available) of the selected ds18b20's scratchpad 
    for (int i = 0; i < 9; i++) data[i] = ds18b20_onewire.read();
      // check the crc
    crc_error = (data[8] != OneWire::crc8(data, 8));  


//TESTING Debug Code for CRC --------------------
// All of this code simply prints out CRC failures and their successful resolutions.   The failing CRC data can be compared
// to the passing CRC data...its a simple logic analyzer for OneWire CRC failures...
// this can be commented out if there is no interest in seeing the CRC errors if they occur
    float temperature;
    temperature = ((int16_t)((data[1] << 8) | (data[0] & DS18B20_RES_MASK)))/16*1.8+32;
    if (crc_error) crc_error_count_DS18B20++;  

    if (crc_error && crc_retries <= DS18B20_CRC_RETRIES) 
      Serial.printlnf("  CRC err #%02d:  %02x %02x %02x %02x %02x %02x %02x %02x %02x  device: %02d temp: %0.1f", 
          (crc_retries+1),data[8],data[7], data[6], data[5], data[4], data[3], data[2], data[1],data[0],ptr,temperature);
    else if (!crc_error && (crc_retries > 0)) {
      Serial.printlnf("  Actual Data:  %02x %02x %02x %02x %02x %02x %02x %02x %02x  device: %02d temp: %0.1f", 
          data[8],data[7], data[6], data[5], data[4], data[3], data[2], data[1],data[0],ptr, temperature);
      Serial.println();
      }
    else if (crc_error) Serial.println();
//TESTING ----------------------------


  } while ((crc_error && (crc_retries++ < DS18B20_CRC_RETRIES)));

    // if the temperature conversion was successfully read, pass it back...else return the CRC FAIL value 
  return (int16_t) (crc_error ?  DS18B20_FAIL_CRC_VALUE : ((data[1] << 8) | (data[0] & DS18B20_RES_MASK)));
}



//
// Publishes the status, in my case: specifically sends it to a Google spreadsheet and the PoolController Android app
// Formatting (using snprintf) changes as per Scruff recommendation
bool publishAllStatus() {
  const char gsSheet[] = "temp";  // google sheet page
  char stats[622];  // place holder for now

snprintf(stats, sizeof(stats),
      "{\"T0\":%.1f"
        ",\"T1\":%.1f"
        ",\"T2\":%.1f"
        ",\"T3\":%.1f"
        ",\"T4\":%.1f"
        ",\"CC\":%ld"
        ",\"ERR_CRC\":%ld"
        ",\"ERR_CRCF\":%ld"
      "}",
      f_current_temps[0],
      f_current_temps[1],
      f_current_temps[2],
      f_current_temps[3],
      f_current_temps[4],
      conversion_count_DS18B20,
      crc_error_count_DS18B20,
      crc_fail_count_DS18B20
    );

  return publishNonBlocking(gsSheet, stats);
}



// A wrapper around Partical.publish() to check connection first to prevent
// blocking. The prefix "pool-" is added to all names to make subscription easy.
// "name" is the "sheet" name within the google spreadsheet that this is being sent to
bool publishNonBlocking(const char* sheet_name, const char* message) {
    const char evtPrefix[] = "pool-";
    char evtName[sizeof(evtPrefix) + strlen(sheet_name)];

    snprintf(evtName, sizeof(evtName), "%s%s", evtPrefix, sheet_name);
    // TODO replace with a failure queue?
    if (Particle.connected()) {
        bool success = Particle.publish(evtName, message, PUBLIC); // TODO, need to understand ramifications of making this PRIVATE
//        Serial.printlnf("Published \"%s\" : \"%s\" with success=%d",
//                        name.c_str(), message.c_str(), success);
        return success;
    } else {
//        Serial.printlnf("Published \"%s\" : \"%s\" with success=0 no internet",
//                        name.c_str(), message.c_str());
    }
    return false;
}