Hello all..
This is my first try with the arduino.. So I did like this... I know i have to clean the code some but it works with one sensor will try several in the weekend any comments?
Obvious errors?
I use the skip as you said would be faster..
#include <OneWire.h>
const byte one_wire_pin = 3;
const byte number_of_sensors = 1;
byte state;
byte last_state;
unsigned long time_t1; //Used to calculate difference between t2-t1 where t2 is millis()
byte sensor_adresses[number_of_sensors][8];
float sensor_temperatures[number_of_sensors];
byte total_discovered_sensors;
byte k;
OneWire one_wire(one_wire_pin);
void setup() {
Serial.begin(9600);
state = 0;
last_state = 0;
Serial.print("Program version 2015-04-15\n");
delay(250);
}
void loop() {
switch (state) {
case 0: //Search if there are any devices
if (last_state != state)
{
last_state = state;
time_t1 = millis();
}
total_discovered_sensors = discover_devices();
if (total_discovered_sensors == 0)
{
state = 99;
}
else
{
state = 1;
}
break;
case 1: //Start a reading and wait for done
if (last_state != state)
{
last_state = state;
time_t1 = millis();
}
one_wire.reset();
one_wire.skip();
one_wire.write(0x44);
if (!one_wire.read()) //Needed to add this as it failed otherwise
{
state = 2;
}
else if ((one_wire.read()) || ((millis() - time_t1) > 750))
{
state = 3;
Serial.println();
Serial.println((millis() - time_t1));
}
break;
case 2: //Busy reading
if (last_state != state)
{
last_state = state;
time_t1 = millis();
}
if ((one_wire.read()) || ((millis() - time_t1) > 750))
{
Serial.println();
Serial.println((millis() - time_t1));
state = 3;
}
break;
case 3: //Done read temperatures
if (last_state != state)
{
last_state = state;
time_t1 = millis();
}
k = 0;
while (k < total_discovered_sensors) {
sensor_temperatures[k] = read_temperatures(one_wire, sensor_adresses[k]);
k++;
}
state = 1;
break;
default:
state = 0;
}
}
byte discover_devices() {
byte j = 0;
byte i = 0;
one_wire.reset();
while ((j < number_of_sensors) && (one_wire.search(sensor_adresses[j]))) {
j++;
Serial.print("ROM =");
for ( i = 0; i < 8; i++) {
Serial.write(' ');
Serial.print(sensor_adresses[0][i], HEX);
}
}
one_wire.reset_search();
return j;
}
float read_temperatures(OneWire one_wire, byte addr[8]) {
byte present = 0;
byte i;
byte data[12];
byte type_s;
float celsius;
int16_t raw;
switch (addr[0]) {
case 0x10:
Serial.println(F(" Chip = DS18S20")); // or old DS1820
type_s = 1;
break;
case 0x28:
Serial.println(F(" Chip = DS18B20"));
type_s = 0;
break;
case 0x22:
Serial.println(F(" Chip = DS1822"));
type_s = 0;
break;
default:
Serial.println(F("Device is not a DS18x20 family device."));
}
present = one_wire.reset();
one_wire.select(addr);
one_wire.write(0xBE); // Read Scratchpad
for ( i = 0; i < 9; i++) { // we need 9 bytes
data[i] = one_wire.read();
}
// convert the data to actual temperature
raw = (data[1] << 8) | data[0];
if (type_s) {
raw = raw << 3; // 9 bit resolution default
if (data[7] == 0x10) {
// count remain gives full 12 bit resolution
raw = (raw & 0xFFF0) + 12 - data[6];
}
else {
byte cfg = (data[4] & 0x60);
if (cfg == 0x00) raw = raw << 3; // 9 bit resolution, 93.75 ms
else if (cfg == 0x20) raw = raw << 2; // 10 bit res, 187.5 ms
else if (cfg == 0x40) raw = raw << 1; // 11 bit res, 375 ms
// default is 12 bit resolution, 750 ms conversion time
}
}
celsius = (float)raw / 16.0;
Serial.print(" Temperature = ");
Serial.println(celsius);
return celsius;
}
Quote from: hence.persson on April 15, 2015, 05:46:48 PM
Hello all..
This is my first try with the arduino.. So I did like this... I know i have to clean the code some but it works with one sensor will try several in the weekend any comments?
Obvious errors?
I use the skip as you said would be faster..
<snip>
const byte number_of_sensors = 1;
<snip...>
Welcome! Taking on 'one wire' as your first Arduino project is courageous ;)
I did a very quick skim and didn't see anything obvious, but, I do struggle with why you're making this specific declaration as 'const variable' (seeming contradiction in terms, but exactly what you're doing). ISTM some compilers would choke on subsequently using this to declare the size of your arrays. Why not just #define MAX_NUM_SENSORS and use this throughout your code. I think this will be more readable and meaningful to experienced programmers plus it's straightforward for the compiler to use ldi instructions if it is a literal. I have the same opinion on using 'variables', const or otherwise, as 'pin' definitions. If the pin is constant throughout the program, use the straightforward convention of defining it literally rather than taking up a RAM storage location that needs to be fetched.
Don't take this as a criticism, it's simply my opinion on programming practice. Like I said, welcome to the mix. Please let us know how you make out with multiple sensors. The OneWire library supports this fairly well so if you have any problems, come back to us, we'll help you out.
Tom
Thanks for the welcome!
I am from the PLC world using ladder and ST (structured text).. There you only have constants and no fancy define etc.. Thats why I used const.. But if Define is cleaner and more wide-spread then I will make the changes to use that instead.. When it works with several sensors and I have cleaned up the code the best I can I will post the results here.. Now its time to scroll through the many other interesting topics..
Quote from: hence.persson on April 15, 2015, 07:46:45 PM
Thanks for the welcome!
I am from the PLC world using ladder and ST (structured text).. There you only have constants and no fancy define etc.. Thats why I used const.. But if Define is cleaner and more wide-spread then I will make the changes to use that instead.. When it works with several sensors and I have cleaned up the code the best I can I will post the results here.. Now its time to scroll through the many other interesting topics..
Are you looking for someone to 'bleed' on your code so that you can see how 'others would do it'? Given that you have working code right out of the box (with fairly clean code, I'll add), I'd say you're in pretty good shape.
The nice thing about Arduino is that it's fairly quick turn to go from a new idea to a 'try' at an implementation. This significantly increases the learning rate. The bad thing is there is no decent debugger, so you learn rather crude debugging methods. Still, there are a LOT of successful people out there who learned on exactly this 'methodology'...
Have fun!
Tom
Quote from: TomWS on April 15, 2015, 09:03:00 PM
Quote from: hence.persson on April 15, 2015, 07:46:45 PM
Thanks for the welcome!
I am from the PLC world using ladder and ST (structured text).. There you only have constants and no fancy define etc.. Thats why I used const.. But if Define is cleaner and more wide-spread then I will make the changes to use that instead.. When it works with several sensors and I have cleaned up the code the best I can I will post the results here.. Now its time to scroll through the many other interesting topics..
Are you looking for someone to 'bleed' on your code so that you can see how 'others would do it'? Given that you have working code right out of the box (with fairly clean code, I'll add), I'd say you're in pretty good shape.
The nice thing about Arduino is that it's fairly quick turn to go from a new idea to a 'try' at an implementation. This significantly increases the learning rate. The bad thing is there is no decent debugger, so you learn rather crude debugging methods. Still, there are a LOT of successful people out there who learned on exactly this 'methodology'...
Have fun!
Tom
Just wanted input if I was doing anything wrong and if not someone could use my code as there wasn't any example using the skip command to start conversion for all sensors..
As for optimizing the DS18B20 read I have one problem.. My state 3 (to read the already converted temperatures) takes ~50ms to finish which would mean if I have ten sensors I would stay in that state for 500ms and then the best update time i would have (if using 12bit resolution) would be ~1,1s+
Also if using my code as is for every sensor you have you would lock up the controller for 50ms * number of sensors each time state 3 is activated.. I changed the code so that will not happen so the controller can do other things in the time it takes to read the temperatures.
Will try with multiple DS18B20 in the weekend to confirm my suspicions.
But as I am a beginner with this maybe its just me doing something wrong but I don't think so but feel free to prove me wrong.
Is it ok to continue in this thread or should we split my experiments from this?
I've a string of DS18B20s that I will try this on this weekend. I'll let you know how it turns out. Been meaning to take another look at this for a while.
C
Quote from: ColinR on April 17, 2015, 04:07:23 AM
I've a string of DS18B20s that I will try this on this weekend. I'll let you know how it turns out. Been meaning to take another look at this for a while.
C
Colin, if your goal is to verify that the OneWire library works with multiple DS18B20s, then to save you the effort, I can personally verify that it does. It works fine, no issues.
I'm not sure about hence.persson's timing concern, it's been a while since I've tested this so I'm not sure what he's referring to. The one thing, which I am sure you're familiar, is you basically have to give it enough time to convert. That you can send the command to all at once is essential, and, as you pointed out, watch for current surges if using parasitic power.
Tom
Quote from: TomWS on April 17, 2015, 07:59:24 AM
Quote from: ColinR on April 17, 2015, 04:07:23 AM
I've a string of DS18B20s that I will try this on this weekend. I'll let you know how it turns out. Been meaning to take another look at this for a while.
C
Colin, if your goal is to verify that the OneWire library works with multiple DS18B20s, then to save you the effort, I can personally verify that it does. It works fine, no issues.
I'm not sure about hence.persson's timing concern, it's been a while since I've tested this so I'm not sure what he's referring to. The one thing, which I am sure you're familiar, is you basically have to give it enough time to convert. That you can send the command to all at once is essential, and, as you pointed out, watch for current surges if using parasitic power.
Tom
The problem is that the conversion takes ~600ms (no problem as i don't lock up the controller) to do but for some reason the reading of temperatures takes ~50ms (the controller is locked this time).
I know that multiple readings should work but haven't seen any example use the skip command to start conversion for all sensors at the same time. My code does that but I haven't confirmed that my code does work with several.
Quote from: hence.persson on April 17, 2015, 10:31:34 AM
<snip>
The problem is that the conversion takes ~600ms (no problem as i don't lock up the controller) to do but for some reason the reading of temperatures takes ~50ms (the controller is locked this time).
I know that multiple readings should work but haven't seen any example use the skip command to start conversion for all sensors at the same time. My code does that but I haven't confirmed that my code does work with several.
UPDATE:
Here's what I use...
For variables & objects:
OneWire ds(ONE_WIRE_PIN); // on pin 5 (a 4.7K resistor is necessary)
typedef
struct ONE_WIRE_INFO_TYPE
{
byte
addr[8],
data[9],
type_s; // the type of sensor (old or new format)
} OneWireInfo;
OneWireInfo owProbes[8]; // support up to 8 probes
byte
numProbes=0;
Inside setup():
... INSIDE setup()...
if (0==findOWprobes()) // find all the one wire probes on our link
Serial.println("No One Wire Probes Found!");
...
Inside loop():
...
// now gather OneWire probe data (if there are any)
if (numProbes)
{
readOWprobes();
}
...
And, finally, the functions I use:
//===================================================================================
// owFuncs.c - implementation file one wire wrapper functions
//===================================================================================
int findOWprobes(void)
{
char dbgbuf[128];
byte
i,
present = 0,
type_s;
for (numProbes=0; numProbes < 8; )
{
// first initialize the data block
memset(&owProbes[numProbes], 0, sizeof(OneWireInfo));
if ( !ds.search(owProbes[numProbes].addr)) {
Serial.println("No more One Wire addresses.");
Serial.println();
ds.reset_search();
delay(250);
return numProbes;
}
int len = sprintf(dbgbuf,"ROM =");
for( i = 0; i < 8; i++) {
len += sprintf(&dbgbuf[len]," %02X", owProbes[numProbes].addr[i]);
}
if (OneWire::crc8(owProbes[numProbes].addr, 7) != owProbes[numProbes].addr[7]) {
len += sprintf(&dbgbuf[len],"CRC is not valid!\n");
Serial.print(dbgbuf);
continue; // ignore this probe
}
Serial.println(dbgbuf);
// the first ROM byte indicates which chip
switch (owProbes[numProbes].addr[0]) {
case 0x10:
Serial.println(" Chip = DS18S20"); // or old DS1820
owProbes[numProbes].type_s = 1;
break;
case 0x28:
Serial.println(" Chip = DS18B20");
owProbes[numProbes].type_s = 0;
break;
case 0x22:
Serial.println(" Chip = DS1822");
owProbes[numProbes].type_s = 0;
break;
default:
Serial.println("Device is not a DS18x20 family device.");
continue;
}
numProbes++; // found a good one, move on to the next
}
return numProbes;
}
void readOWprobes(void)
{
char dbgbuf[128];
byte
i,
len,
present = 0;
ds.reset();
ds.write(0xcc); // issue SKIP_ROM command so ALL probes convert at once!
ds.write(0x44, 1); // start conversion, with parasite power on at the end
delay(1000); // maybe 750ms is enough, maybe not
// we might do a ds.depower() here, but the reset will take care of it.
for (byte i=0; i<numProbes; i++)
{
present = ds.reset();
ds.select(owProbes[i].addr);
ds.write(0xBE); // Read Scratchpad
if (verboseOn)
len = sprintf(dbgbuf,"Probe[%d]:%d=",i,present);
for( int j = 0; j < 9; j++) {
owProbes[i].data[j] = ds.read();
if (verboseOn) len += sprintf(&dbgbuf[len]," %02X", owProbes[i].data[j]);
}
if (verboseOn)
{
Serial.print(dbgbuf);
Serial.print(" CRC=");
Serial.print(OneWire::crc8(owProbes[i].data, 8), HEX);
Serial.println();
}
}
}
Have fun. I've tested this with up to four probes.
Tom
Quote from: TomWS on April 17, 2015, 07:59:24 AM
Quote from: ColinR on April 17, 2015, 04:07:23 AM
I've a string of DS18B20s that I will try this on this weekend. I'll let you know how it turns out. Been meaning to take another look at this for a while.
C
Colin, if your goal is to verify that the OneWire library works with multiple DS18B20s, then to save you the effort, I can personally verify that it does. It works fine, no issues.
I'm not sure about hence.persson's timing concern, it's been a while since I've tested this so I'm not sure what he's referring to. The one thing, which I am sure you're familiar, is you basically have to give it enough time to convert. That you can send the command to all at once is essential, and, as you pointed out, watch for current surges if using parasitic power.
Tom
Well, I'd like to test the timing, and that I can write nice, clean, optimized code to perform these functions. If nothing else, I've learned from this exercise that the code that's been written to perform these operations has, for the most part, been written by people unfamiliar with the idiosyncrasies of OneWire communication, often coupled with very inefficient memory usage. These things aren't really an issue if you have low sensor count, and time and memory to burn. I don't have the last two, and prefer to write the cleanest code I can the first time so I don't have to come back and reintroduce myself to it repeatedly.
I see that you've written some code, which I'll take a look at first.
Colin
Hello now i made some minor changes to my own code (had som bug when using more than one sensor) and tested and these were the results with two sensors..
282ms in state 0 (Search)
3ms in state 1 (Start convert)
621ms in state 2 (Waiting for conversion done)
40ms in state 3 (Read temperatures and print to screen) //This is the same with two or one sensor..
So fresh readings from two sensors in 664ms... With 12bit resolution..
Apart from the search state the longest lock of the controller is 40ms in state 3 same time with one or two sensors would be interesting to know what does that. Else i have a scantime of 1ms or so..
Tom will look at your code and maybe use that instead but I see that you have used a 1000ms delay to wait for the readings and not doing as Colin first stated -- Wait for the readings to get ready and not a ms more.. Also the controller isnt usable to other things while waiting for results..
Quote from: hence.persson on April 17, 2015, 04:58:14 PM
<snip>
Tom will look at your code and maybe use that instead but I see that you have used a 1000ms delay to wait for the readings and not doing as Colin first stated -- Wait for the readings to get ready and not a ms more.. Also the controller isnt usable to other things while waiting for results..
That code was quickly snipped from some I had hanging around and was mainly to show you how to read multiple devices. The delay you're referring to can be eliminated totally with the right program structure (as you say, doing something else while converting - in my case reading the probes was the only thing I wanted to do...). I make NO WARRANTY claims on that code ;)
I'm glad you got your code working,
Tom