DS18B20 ROM reads alternate bytes

Go To Last Post
41 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I am working with the popular DS18B20 temperature sensor, connected to ATMega328P (16MHz). Apart from the temperature readings, I want to read the ROM code an entire scratchpad. The code snippet is as shown. I have also attached the snapshot of the Realterm screen

I am able to read the entire 9 bytes of the scratchpad, the proper ambient temperature and displayed on realterm. 

Problem is that the READROM code is reading just alternate bytes of the ROM, as can be seen. Just once, initially I was able to read the ROM code properly. It was shown as 28 10 88 14 F3 3D BB 08. it is reversed, but no issues with that. All subsequent reads are as displayed in realterm: 28 88 F3 BB 05. Note that the CRC has changed from 08 to 05. 

The codes for READROM and READSCRATCHPAD are same.

Can anyone please assist? 

The code snippet is from main.c 

#include <avr/io.h>
#include <stdio.h>
#include <string.h>
#include <stdint.h>
#include <avr/interrupt.h>
#include <util/delay.h>

#include "ds18b20.h"

//function declarations
void UART_init();
void UART_SendString(char *str);	//send a set of char's to UART terminal
void int16toHex(uint16_t n, char *ptr);	//convert a 16 bit number to ASCII char's
uint16_t ds18b20_gettemp();
void tstostr(uint16_t ts, char *s);		//convert a 16 bit number to ASCII char's
void reverse_string(char *s);	//reverse a byte string after ASCII conversion (if required)
void hex2a(uint8_t *s, char *buff); 	//convert an array of hex bytes to ASCII char's

//Function to print ASCII conversion of hex bytes
hex2a(uint8_t*s, char*buff)
{
	static volatile uint8_t d = 0; //some variable
	do
	{
		d = *s;
		d &= 0xf0;			//take high nibble
		d = d>>4;			//shift to low nibble
		if (d <= 9)
		{
			*buff++ = d + '0';
		}
		else
		{
			*buff++ = (d-0xA) + 'A';
		}
		d = *s;
		d &= 0x0f;			// take low nibble
		if (d <= 9)
		{
			*buff++ = d + '0';
		}
		else
		{
			*buff++ = (d-0xA) + 'A';	//d-10+'A'
		}
		s++;
	}
	while (*s != '\0');
	
	*buff = '\0';
}


tstostr(uint16_t ts, char *s)
{
	int8_t t, neg;
	char *p = s;

	// integer part processing
	//
	t = (int8_t)(ts >> 4);
	neg = t & 0x80;
	if (neg)
	t = ~t;
	do
	{
		*p++ = t % 10 + '0';
	} while ((t /= 10) > 0);


	*p++ = neg ? '-' : '+';

	*p = '\0';
	reverse_string(s);

	// fractional part processing
	//
	*p++ = '.';
	t = (int8_t)ts & 0xf;	//use only LSB of temperature reading
	if (neg)
	t = (-t) & 0xf;
	t = ((int16_t)t * 100) >> 4;   //* 100 for 2 dec places, *1000 fo 3 dec places. Then /16 (since hex => base 16) 
	if ((t % 10) >= 5)           // Round off to 1 dec place
		t += 10;
	t /= 10;					// shift on 1 decimal digit
	*p++ = (char)t + '0';

	*p = '\0';
}



int main(void) 
{
	uint8_t dsbuff[10];			//buffer to hold DS18B20 data
	uint8_t *ds = dsbuff; 
//	char scratchpadbuffer[20];	//buffer to hold scratchpad data
	char printbuff[20];			// general use display buffer 
	char *buff = printbuff;		// pointer returned by function for printing
//	char d = 0;	
	uint8_t j = 0;					// some temp location
	
	//init uart
	UART_init();	//init uart

	//init interrupt
	sei();
	
	UART_SendString("DS18B20 Temperature Probe\r\n");
	
	//preliminary info -just to know//
	ds18b20_reset();	// DS initialise
	ds18b20_writebyte(DS18B20_CMD_READROM);	//read ROM - get 8 bytes
	for(j=0; j<8; j++)
	{
		dsbuff[j] = ds18b20_readbyte(); //reads LSB of DS family code (28) first, CRC (MSB) last <== reads alternate bytes in ROM
	}
	ds18b20_reset();		// reset after ROM command
	dsbuff[8] = '\0';		//add null 

	hex2a(ds, buff);
	
	UART_SendString("ROM code: ");
	UART_SendString((char *) ds);
	UART_SendString("\r\n");	
	UART_SendString(buff); 
	UART_SendString("\r\n");
	
	//read scratchpad - 9 bytes - use scratchpad buffer
	ds18b20_reset();
	ds18b20_writebyte(DS18B20_CMD_SKIPROM); //skip ROM
	ds18b20_writebyte(DS18B20_CMD_RSCRATCHPAD);	//read ROM - get 8 bytes
	for(j=0; j<9; j++)
	{
		dsbuff[j] = ds18b20_readbyte(); //reads 9 bytes of scratchpad  <== reads all bytes properly
	}
	ds18b20_reset();		// reset after ROM command
	dsbuff[9] = '\0';		//add null
	
	hex2a(ds, buff);
	
	UART_SendString("Scratchpad: ");
	UART_SendString((char *) ds);
	UART_SendString("\r\n");
	UART_SendString(buff);
	UART_SendString("\r\n");


	//ds18b20_reset();
	//skiprom
	//write scratchpad - 3 bytes -set Th, Tl, Resolution
	
	//ds18b20_reset();
	//skiprom
	//copy scratchpad to ROM - 3 bytes
	//delay 20ms 
	

	while(1)
	{
		rawtemp = ds18b20_gettemp();	// function returns double temp

		tstostr(rawtemp, buff);	//convert 16bit raw temp to printable ASCII
	
		UART_SendString(buff);
		UART_SendString("\n""\r");
		
	/////////////// read raw temperature nibbles //////////////////

		int16toHex(rawtemp, buff);
		UART_SendString(buff);
		UART_SendString("\n""\r");
//		UART_SendString("\r");
	
		_delay_ms(5000);
	}
	
	return 0;
}

This topic has a solution.
Last Edited: Thu. Apr 15, 2021 - 02:08 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hello. A bit surprising hat no advice has been given for a solution to the above issue. 

I tried placing this statement just before the (CMD_READROM) statement. Now I am able to get all 8 bytes but these are all FFh.   When commented out this SKIPROM statement, back to reading only 5 bytes. I noticed that the power on temperature reading n  the DS18B20 scratchpad is not 0x55h, but it is the last ambient reading when temperature conversion was done. Can this be a clone of the real DS18B20?

Any help will be appreciated.

ds18b20_writebyte(DS18B20_CMD_SKIPROM); //skip ROM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

This may be a separate issue, but your use of

hex2a(uint8_t*s, char*buff)

is strange.

s is pointer to array of uint8_t (with 8 or 9 entries by the looks of it), and you loop until you get a zero value in s.

What is to say one of those 8 or 9 entries won't be 0?

Looping until you get a zero is perfectly normal when the array represents a null terminated string, as you end up with in buff, but not when it is array of arbitrary data.

It would be more normal to pass in a length to say how many entries from s are to be processed.

Or just convert one uint8_t at a time.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

Hello,

 

in Your code I cannot see the #define off DS18B20_CMD_READROM. This must be 0x33. Please check this

(datasheet page 13)

 

The Timing

I cannot see the code behind the Read/Write functions.

Please check the correct Read/Write time slots on 1 wire bus. (datasheet page 15)

 

A transmission off one byte must be finished before You can start the next Read command.

(delays may be helpfull)

 

The best way to check this is a trace off the bus with a digital storage oscilloscope. 

 

regards 

EllenR

 

 

 

 

 

 

 

 

 


 

Attachment(s): 

Senior Electric Engineer
--------------------------
I use Atmega128, AS7, GNU GCC, Atmel ICE

Last Edited: Sat. Apr 10, 2021 - 10:59 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

EllenR wrote:
The best way to check this is a trace off the bus with a digital storage oscilloscope.

+1

 

A useful feature of the Maxim 1-WireTM bus is that the timing between bits ("time slots")  is non-critical - so you can put breakpoints in your code and capture & examine each bit being transmitted/received on your scope.

 

Some key documents for working with the 1-WireTM bus: https://www.avrfreaks.net/commen...

 

 

 

 

 

 

 

 

 

 

 


 

Top Tips:

  1. How to properly post source code - see: https://www.avrfreaks.net/comment... - also how to properly include images/pictures
  2. "Garbage" characters on a serial terminal are (almost?) invariably due to wrong baud rate - see: https://learn.sparkfun.com/tutorials/serial-communication
  3. Wrong baud rate is usually due to not running at the speed you thought; check by blinking a LED to see if you get the speed you expected
  4. Difference between a crystal, and a crystal oscillatorhttps://www.avrfreaks.net/comment...
  5. When your question is resolved, mark the solution: https://www.avrfreaks.net/comment...
  6. Beginner's "Getting Started" tips: https://www.avrfreaks.net/comment...
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hello,

 

I have found another issue in Your code:

 

Before You read out temperature You have to perform a Convert Command:  0x44

 

This forces the device to generate a new temperature value into the registers Thl and Thh. 

 

By missing this You will always read the same value. After power on this is 85°C.

 

After Convert Command  the device needs some time,  Temperature Conversion Time depending on the programmed solution

(see table page 3) 

 

The next question is powering the device. (page 7)

Do You use External or Parasite Power supply?

 

 

regards 

EllenR

 

Senior Electric Engineer
--------------------------
I use Atmega128, AS7, GNU GCC, Atmel ICE

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

MrKendo wrote:

This may be a separate issue, but your use of

hex2a(uint8_t*s, char*buff)

is strange.

s is pointer to array of uint8_t (with 8 or 9 entries by the looks of it), and you loop until you get a zero value in s.

What is to say one of those 8 or 9 entries won't be 0?

Looping until you get a zero is perfectly normal when the array represents a null terminated string, as you end up with in buff, but not when it is array of arbitrary data.

It would be more normal to pass in a length to say how many entries from s are to be processed.

Or just convert one uint8_t at a time.

Point noted, MrKendo. I should use the length of the array as a terminator or some other means, not check for '\0', which is 0x00. This is best practice & I will modify the code accordingly. However, if the function terminates at the first 0, how does it go on to convert the bytes till the CRC? As can be seen, 5 bytes are displayed, the LSB with the DS18B20 family code (0x28), the MSB as the CRC. So no byte is missed during conversion. 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

EllenR wrote:

Hello,

 

in Your code I cannot see the #define off DS18B20_CMD_READROM. This must be 0x33. Please check this

(datasheet page 13)

 

Hello EllenR. The code for DS18B20_CMD_READROM is 0x33. This is in a header file which I have not posted to ensure no clutter for all. All timings are proper, since I am able to read the scratchpad and the temperature properly.

 

 

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

EllenR wrote:

Hello,

 

I have found another issue in Your code:

 

Before You read out temperature You have to perform a Convert Command:  0x44

 

This forces the device to generate a new temperature value into the registers Thl and Thh. 

 

By missing this You will always read the same value. After power on this is 85°C.

 

After Convert Command  the device needs some time,  Temperature Conversion Time depending on the programmed solution

(see table page 3) 

 

The next question is powering the device. (page 7)

Do You use External or Parasite Power supply?

 

 

regards 

EllenR

 

Temperature conversion command (0x44) is given. Please see the code snippet below. The conversion is checked to be completed when the DS returns a bit. 

uint16_t ds18b20_gettemp() 
{
	uint8_t temperature_l;
	uint8_t temperature_h;
	int16_t temperature;
//	double retd = 0;
	
	#if DS18B20_STOPINTERRUPTONREAD == 1
	cli();
	#endif

	ds18b20_reset(); //reset
	ds18b20_writebyte(DS18B20_CMD_SKIPROM); //skip ROM
	ds18b20_writebyte(DS18B20_CMD_CONVERTTEMP); //start temperature conversion

	while(!ds18b20_readbit()); //wait until conversion is complete

	ds18b20_reset(); //reset
	ds18b20_writebyte(DS18B20_CMD_SKIPROM); //skip ROM
	ds18b20_writebyte(DS18B20_CMD_RSCRATCHPAD); //read scratchpad

	//read 2 byte from scratchpad
	temperature_l = ds18b20_readbyte();
	temperature_h = ds18b20_readbyte();
	ds18b20_reset(); //reset
	
	temperature = ( ( temperature_h << 8 ) + temperature_l );

	rawtemp = temperature;		//for debugging
		
	//check for -ve temperature readings
	//temperature is a signed 16 bit int, bit 15 =1, so ANDing with 80h will always be !0
	if(temperature_h & 0x80)
	{
		temperature = temperature - 1;	//convert using two's complement
		temperature = ~temperature;
	}
	
	#if DS18B20_STOPINTERRUPTONREAD == 1
	sei();
	#endif

	return rawtemp;
	//convert the 12 bit value obtained
//	retd = ( ( temperature_h << 8 ) + temperature_l ) * 0.0625;
//	retd = (temperature *625)/10000;
//	return retd;
}

Anyway, I am not concerned about the temperature reading here, since I am getting proper readings. More bothering is why the READROM does not send to all 9 bytes, skips bytes 1,3,5 as can be seen in the realterm screen. This is my concern which I have not been able to resolve.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

EllenR wrote:

 

The next question is powering the device. (page 7)

Do You use External or Parasite Power supply?

 

 

External 5V power supply with 4K7 ohms pullup, as mentioned in the datasheet. This is a standard off the shelf temperature sensor with 1.2 m, 3 core cable.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

amorawala wrote:
More bothering is why the READROM does not send to all 9 bytes, skips bytes 1,3,5 as can be seen in the realterm screen. This is my concern which I have not been able to resolve.

 

You tell me You want reading 9 bytes ROM. The lenght is 64bit >> 8 bytes. The posted code above ist correct. 

 

I'm working about 10 years with this sensors. I've experience. 

Hmm he skips byte 1,3,5 ?? That is strange.

 

What is the content off dsbuff after the for loop? Place a breakpoint here and add this array to the watch window. 

 

I think You have to analyse the 1 wire bus with a scope. I have no other idea.

 

regards 

EllenR

 

P.S. is this Your Code?

I have my own code and don't use any library

 

 

 

 

Senior Electric Engineer
--------------------------
I use Atmega128, AS7, GNU GCC, Atmel ICE

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

amorawala wrote:
External 5V power supply with 4K7 ohms pullup

 

4k7 for the data line? 

 

try this: power the VSS pin off the sensor to +5V. He needs a lot energy when you read 8 bytes. 

The other way is to perform a strong pull up. take a look at operation example page 18. 

 

 

Senior Electric Engineer
--------------------------
I use Atmega128, AS7, GNU GCC, Atmel ICE

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

EllenR wrote:

amorawala wrote:
More bothering is why the READROM does not send to all 9 bytes, skips bytes 1,3,5 as can be seen in the realterm screen. This is my concern which I have not been able to resolve.

 

You tell me You want reading 9 bytes ROM. The lenght is 64bit >> 8 bytes. The posted code above ist correct. 

 

I'm working about 10 years with this sensors. I've experience. 

Hmm he skips byte 1,3,5 ?? That is strange.

 

What is the content off dsbuff after the for loop? Place a breakpoint here and add this array to the watch window. 

 

I think You have to analyse the 1 wire bus with a scope. I have no other idea.

 

regards 

EllenR

 

P.S. is this Your Code?

I have my own code and don't use any library

 

Mistake on my part. The ROM code is 8 bytes, which I want to read.

Please see the realterm screen shot. The dsbuff is displayed as raw data just after the opening line "DS18B20 Temperature probe". It is adjacent to the prefix "ROM code:". it is shown as unprintable char's but can make out just 5 bytes before the CRLF.

I do not have an in circuit debugger, so cannot debug exhaustively as you mention.

I have used the code from David Gironi: 

ds18b20 lib 0x02

copyright (c) Davide Gironi, 2012

Released under GPLv3.
 

Modified the conversion part for temperature reading and added read rom, read scratchpad etc., to understand the DS probe in depth. I plan to change configuration to 12 bit, 10 bit and see the effects. At present it is 9 bit resolution, can be seen from the scratchpad byte 4.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

EllenR wrote:

amorawala wrote:
External 5V power supply with 4K7 ohms pullup

 

4k7 for the data line? 

 

try this: power the VSS pin off the sensor to +5V. He needs a lot energy when you read 8 bytes. 

The other way is to perform a strong pull up. take a look at operation example page 18. 

 

 

4K7 is the resistor shown in te datasheet, page 7, fig 7 - "powering DS18B20 with external power supply". however, this value gives about 1.2mA when converting / reading. I will try with 2K7. this will give about 2.2 mA or with 3K3 to give about 1.5mA. The datasheet specifies a max sink of 1.5mA when conversion or reading. So I can stick to 3K3.

Page 18 operational example are shown for parasitic power, which I am not using.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

amorawala wrote:

I have used the code from David Gironi: 

ds18b20 lib 0x02

copyright (c) Davide Gironi, 2012

Released under GPLv3.

 

OK, 

I've done an extraordinary work for You. I started my machines. 

Actual I work on a project with DS2408 (I/O expander). The ROM read routines are identically.

On this trace you can see a wire reset followed by command 0x33. 

The the master then reads 8 bytes. This is the ROM code.

 

Please watch the timing. There is a lot time between the bytes.

May be too much? I'm interessed in Your trace

 

regards 

Ellen R

 

 

 

 

 

 

 

Attachment(s): 

Senior Electric Engineer
--------------------------
I use Atmega128, AS7, GNU GCC, Atmel ICE

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

 

 

OK, 

I've done an extraordinary work for You. I started my machines. 

Actual I work on a project with DS2408 (I/O expander). The ROM read routines are identically.

On this trace you can see a wire reset followed by command 0x33. 

The the master then reads 8 bytes. This is the ROM code.

 

Please watch the timing. There is a lot time between the bytes.

May be too much? I'm interessed in Your trace

 

regards 

Ellen R

 

Many thanks Ellen for the efforts yo are putting in. I have a 2 channel digital scope. But I do not have a debugger to be able to put in breakpoints and debug. However, I will try to get some trace by reducing unnecessary code and resetting the MCU. Give me some time since it is late here in Bombay, India. Will do this tomorrow and post the traces. I too am eager to see what is happening. Thanks.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hello,

  If you are using a Mega328p at 16MHz then you should be modeling your application on an Arduino Nano/UNO first.  The UNO/nano has the same AVR configuration and you can use pre-tested, working libraries downloaded from the web.  For this temperature sensor, TI 18B20, I use the downloaded "OneWire.h".  Then I create an object for the IC with:

   OneWire sensorB20(11);     // data from sensor on Arduino D11  (Port B4)

 

Now I use a function from the example file in the OneWire.h download:

 

*************************************************************************************/
float OneWireTemperature() {  // Texas Instruments 18B20 temperature sensor
        byte OneWireAddr[8], OneWireData[12], OneWireDevicePresent = 0, OneWireDeviceType;

        if ( !sensorB20.search(OneWireAddr)) {  // returns with eight IC ROM bytes in OneWireAddr[]
           sensorB20.reset_search();
           delay(250);
        }
        if (OneWire::crc8(OneWireAddr, 7) != OneWireAddr[7]) {
             Serial.println(F("18B20 CRC is not valid!"));  return;  }
             
        Serial.println();
        switch (OneWireAddr[0]) {  // the first ROM byte indicates which chip
             case 0x10: // DS18S2 or old DS1820
               OneWireDeviceType = 1; break;
             case 0x28: // DS18B20
               OneWireDeviceType = 0; break;
             case 0x22: // DS1822
               OneWireDeviceType = 0; break;
             default:   // Device is not a DS18x20 family device
               return;
         }
         
         sensorB20.reset();
         sensorB20.select(OneWireAddr);
         sensorB20.write(0x44, 1);  // start conversion, with parasite power on at the end
         delay(750);
         OneWireDevicePresent = sensorB20.reset();
         sensorB20.select(OneWireAddr);
         sensorB20.write(0xBE);     // Read Scratchpad
         for (uint8_t OneWireIndex = 0; OneWireIndex < 9; OneWireIndex++)
              OneWireData[OneWireIndex] = sensorB20.read();
              
         int16_t rawOneWireData = (OneWireData[1] << 8) | OneWireData[0];
         
         if (OneWireDeviceType) {
              rawOneWireData =  rawOneWireData << 3; // 9 bit resolution default
              if (OneWireData[7] == 0x10)  // "count remain" gives full 12 bit resolution
                     rawOneWireData = ( rawOneWireData & 0xFFF0) + 12 - OneWireData[6];
                
         } else {  // at lower res, the low bits are undefined, so let's zero them
              byte OneWireConfig = (OneWireData[4] & 0x60); // my DS18B20        
              if      (OneWireConfig == 0x00) rawOneWireData = rawOneWireData & ~7; // 9 bit resolution, 93.75 ms
              else if (OneWireConfig == 0x20) rawOneWireData = rawOneWireData & ~3; // 10 bit res, 187.5 ms
              else if (OneWireConfig == 0x40) rawOneWireData = rawOneWireData & ~1; // 11 bit res, 375 ms
         }  // default is 12 bit resolution, 750 ms conversion time
         return (float)  rawOneWireData / 16.0;
}/*%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% */

 

et Voila!  I have a working TI 18b20 prototype that I can experiment with.  

 

Change and adjust code until it doesn't work.  Then go back and undo the last changes until it does work again.

 

Your code doesn't look very modular.  Are you writing it or adapting it from someone else's program?

Don't bother.  Use the Arduino libraries first to get your application working.  Then remove it from the Arduino framework (if necessary).

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Another point not related to your original problem.

Your temperature conversion (unsigned to signed) is suspect.

You have the result already at this point, since temperature is defined as int16_t.

        uint8_t temperature_l;
	uint8_t temperature_h;
	int16_t temperature;

	temperature_l = ds18b20_readbyte();
	temperature_h = ds18b20_readbyte();

	temperature = ( ( temperature_h << 8 ) + temperature_l );

I would actually do it as (to avoid any undefined behaviour with left shift)

	temperature = ((uint16_t)temperature_h << 8) | temperature_l;

this gives you a result on the right hand side (assuming 16 bit int) which is of type uint16_t, but actually represents a signed value in 2's complement form, so you can just assign this to an int16_t.

(No cast to int16_t is needed but wouldn't hurt).

	temperature = (int16_t)(((uint16_t)temperature_h << 8) | temperature_l);

The bit patten is already correct, you just need to interpret it as signed int16_t rather than unsigned uint16_t.

Question is, is it guaranteed always to work like this ie. no change in bit pattern when assigning the uint16_t to int16_t?

Answer is, technically no, but you will have a hard time finding a C compiler where the answer is no.

It stems from the C rules when converting to a signed type.

if the value of the (in this case) uint16_t fits in the range of the signed type (in this cse int16_t), the value is unchanged.

So no problem from 0 to 32767.

If the value doesn't fit (32768 to 65535) then the result is implementation defined ie. the compiler can do what it wants (and is supposed to document somewhere what it will do).

Assuming 2's complement for the signed type however (and stdint signed types like int16_t are required to be 2's complement), the chances of a compiler choosing to do something that does change the bit pattern are pretty much non existent.

For the paranoid, there are various ways you could do it that would be absolutely guaranteed (and would probably be optimised by compiler) if you can be bothered.

 

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

EllenR wrote:

amorawala wrote:

I have used the code from David Gironi: 

ds18b20 lib 0x02

copyright (c) Davide Gironi, 2012

Released under GPLv3.

 

Please watch the timing. There is a lot time between the bytes.

May be too much? I'm interested in Your trace

 

Hello Ellen. I managed to obtain some traces on my DSO (UNI-T, 60MHz, dual ch). Sad to say, these are not what I expect. Just have a look at them. In all these we can see some spikes, I cannot identify these. If I reduce the pullup resistance to 3K3, the bus gets loaded, the data does not switch to (almost) 0v.

I have listed the DSO snapshots as follows:

1. Total-readrom.bmp: I have edited the code to remove all other routines, just having the READROM routine. This simplifies the experiment and I can concentrate on a small part. I have added a reset() routine after all data is read from the ROM. This bmp shows the data between two reset functions. the timings of the reset functions is proper. I have added a 10us recovery time after each write bit.

2. After-reset-write-command-0x33: This bmp shows the command 0x33 being properly sent, LSbit first. Followed by some data received from the DS.

3. Some bits: a snapshot of some bits in between.

4. End-bits-with-reset: This bmp shows the bits received before the reset() routine.

I am concerned about the shabby waveforms. Anyone can help clean up this mess? Ellen, can you please share the ROM read code with which yo get such clean waveforms please?

 

1.                                                                              2.                                                                        3.

    4.

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

amorawala wrote:
connected to ATMega328P
amorawala wrote:

//Function to print ASCII conversion of hex bytes

Aside: It's a 32K micro - why even bother writing your own hex routines when you can simply use sprintf() with %0X formats?

 

In fact why even bother with creating a "buff" and then UART_sendstring()? You can just use printf() after FDEV_SETUP_STREAM.

Last Edited: Mon. Apr 12, 2021 - 09:26 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

amorawala wrote:
I am concerned about the shabby waveforms.

 

Hello amorawala  ,

 

that does not look fine ....

 

I Have made a new trace for You. Fully Read Rom cycle.

In the Zoom window You may see  perfect Write & Read time slots.

Master sends a 0x33 to slave and then performs a read. This is the first byte off the Rom Code (= Family Address) 0x29.

With the DS18B20 You should expect a 0x28. 

 

You want my software?

I'm using a TWI-1wire Bridge Controller (DS2482). This performs the full Maxim 1wire specification. (see schematic)

The great advantage is the protocol on bus runs autonomous. 

 

here  my code

/**             get sensor ROM                */
void DS1820_Read_ROM(uint8_t adr,uint8_t addr[])
{
    DS2482_1wire_Reset(adr);
    DS2482_1wire_WriteByte(adr,READ_ROM);
    
    for (int i = 0; i<8; i++)
    {
        DS2482_1wire_ReadByte(adr);
        _delay_us(OneFrameOnWire);            // wait read byte ready
        addr[i] = DS2482_Get_Dataregister(adr);
    }
}

 

Try delays between the frames 

 

regards EllenR

 

 

 

 

Attachment(s): 

Senior Electric Engineer
--------------------------
I use Atmega128, AS7, GNU GCC, Atmel ICE

Last Edited: Mon. Apr 12, 2021 - 09:31 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

amorawala wrote:
the shabby waveforms.
Two things:

 

1) what is the bandwidth of your scope ?

 

2) How are you grounding the measurement?

 

A scope with a low bandwidth or one where the ground connection for the probe is not "solid" could cause the kind of display you showed even if the signal is actually clear and solid itself.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

clawson wrote:

In fact why even bother with creating a "buff" and then UART_sendstring()? You can just use printf() after FDEV_SETUP_STREAM.

 

I have read in numerous articles that the standard gcc functions like printf(), sprintf() etc., take up a lot of memory. Hence in embedded systems, it was mentioned advisable to use lean functions such as what I have. Please advise on this.

Next, I want to try using the FDEV_SETUP_STREAM and printf(). However, I cannot get some proper documentation on how to use these. Even the gcc manual is a bit confusing. Can you please guide me? I really do want to try using standard functions, since this will save me time and I will have robust, tested code.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

clawson wrote:

 

Two things:

 

1) what is the bandwidth of your scope ?

 

2) How are you grounding the measurement?

 

A scope with a low bandwidth or one where the ground connection for the probe is not "solid" could cause the kind of display you showed even if the signal is actually clear and solid itself.

The DSO that I have has 60MHz bandwidth, 1GS/s sampling rate. I am using the crocodile clip on the probe connected to one of the 0V pins on the breakout board. All 0V pins are connected to the 0V of the power supply input.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

amorawala wrote:
I have read in numerous articles that the standard gcc functions like printf(), sprintf() etc., take up a lot of memory. Hence in embedded systems, it was mentioned advisable to use lean functions such as what I have. Please advise on this.
The costs are basically something like:

 

1) if you use floating point (+-*/): about 700 bytes

 

2) if you use STDIO like sprintf/printf: about 500 bytes

 

3) if you printf and enable and use floating point (%f): about 2000 bytes

 

So, no, in a 32K micro there is not really a huge penalty for these things - you pay the price once and then you can use them as much as you like.

 

Of course if you were programming a 512byte AVR the situation could be quite different!

 

Certainly from 16K up (and perhaps about 8K) up there's no problem using stuff like printf() and it makes your development time/effort a whole lot less as you are using proven/tried/tested tools not reinventing the wheel.

 

Even if flash space is a consideration at least consider using itoa(). The GCC version of this non-standard function takes a 3rd parameter for "radix" which means it can do Hex just as easily as decimal or even binary/octal if you like. The one thing it won't do is padding with leading 0's (as you might get with something like "%04X" in (s)printf etc).

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

EllenR wrote:

amorawala wrote:
I am concerned about the shabby waveforms.

 

I Have made a new trace for You. Fully Read Rom cycle.

 

here  my code

/**             get sensor ROM                */
void DS1820_Read_ROM(uint8_t adr,uint8_t addr[])
{
    DS2482_1wire_Reset(adr);
    DS2482_1wire_WriteByte(adr,READ_ROM);
    
    for (int i = 0; i<8; i++)
    {
        DS2482_1wire_ReadByte(adr);
        _delay_us(OneFrameOnWire);            // wait read byte ready
        addr[i] = DS2482_Get_Dataregister(adr);
    }
}

 

Try delays between the frames 

 

Hello Ellen. The waveforms are very clean. I can see that you have a delay of about 400us between the write 0x33 command and beginning of the read cycle. I can also see a delay of about 1.6ms between reading two bytes. 

I suppose this delay statement _delay_us(OneFrameOnWire); in your code is this 1.6ms? If I understand your code right, first the DS2482 byte is read in some location (adr). This is followed by a delay, then the byte is transferred from (adr) to the array addr[]. Am I correct?

I have read the DS18B20 code directly to an array element. I will put a delay between two reads and check. 

But al the above still do not resolve the issue of the missing bytes. This is my grouse. Could it be a clone DS? Temperature readings are proper. Although on power on, or reading the scratchpad (0xbe) after a reset, I do not see 0x55 (85 deg C) but the correct ambient temperature.

Thanks.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

clawson wrote:

amorawala wrote:
I have read in numerous articles that the standard gcc functions like printf(), sprintf() etc., take up a lot of memory. Hence in embedded systems, it was mentioned advisable to use lean functions such as what I have. Please advise on this.

 

Certainly from 16K up (and perhaps about 8K) up there's no problem using stuff like printf() and it makes your development time/effort a whole lot less as you are using proven/tried/tested tools not reinventing the wheel.

 

Even if flash space is a consideration at least consider using itoa(). The GCC version of this non-standard function takes a 3rd parameter for "radix" which means it can do Hex just as easily as decimal or even binary/octal if you like. The one thing it won't do is padding with leading 0's (as you might get with something like "%04X" in (s)printf etc).

 

This is good information Clawson. I will edit the code to use these standard functions. Thanks.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Just as an example: If I build this:

#include <stdio.h>
#include <avr/io.h>

int uart_put(char c, FILE *stream) {
    while(!(UCSRA & (1 << UDRE)));
    UDR = c;
    return 0;
}

void uart_puts(char * str) {
    while (*str) {
        uart_put(*str++, NULL);
    }
}

int main(void)
{
    while(1)  		/* Repeat forever*/
    {
        uart_puts("hello");
    }
}

I get:

			Program Memory Usage 	:	196 bytes   0.6 % Full

while with:

#include <stdio.h>
#include <avr/io.h>

int uart_put(char c, FILE *stream) {
    while(!(UCSRA & (1 << UDRE)));
    UDR = c;
    return 0;
}

static FILE outStr = FDEV_SETUP_STREAM(uart_put, NULL, _FDEV_SETUP_WRITE);

int main(void)
{
    stdout = &outStr;
    while(1)  		/* Repeat forever*/
    {
        printf("hello");
    }
}

I get:

			Program Memory Usage 	:	1680 bytes   5.1 % Full

So that is the "cost up" of including printf(). However I can now do:

int main(void)
{
    stdout = &outStr;
    int n = 12345;
    unsigned long l = 0xDEADBEEF;
    
    while(1)  		/* Repeat forever*/
    {
        printf("n (dec) = %d, n (hex)=%04X, l (hex) + %08lX", n, n, l);
    }

which prints n in both hex and decimal and l in just hex and the cost only increases to:

			Program Memory Usage 	:	1782 bytes   5.4 % Full

and what's more now I can do all kinds of formatted print output. (of course printf() does not have a format for binary though).

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

clawson wrote:

Just as an example: If I build this:

which prints n in both hex and decimal and l in just hex and the cost only increases to:

			Program Memory Usage 	:	1782 bytes   5.4 % Full

and what's more now I can do all kinds of formatted print output. (of course printf() does not have a format for binary though).

Yes, Clawson. I should now start using these functions. Seems much better than spend my time "reinventing the wheel". I have better things to do, such as solving crazy issues like the present one:)

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

amorawala wrote:
I suppose this delay statement _delay_us(OneFrameOnWire); in your code is this 1.6ms? If I understand your code right, first the DS2482 byte is read in some location (adr). This is followed by a delay, then the byte is transferred from (adr) to the array addr[]. Am I correct?

 

Hello,

yes is correct.  adr is the I2C address off the DS2482. addr[] is a pointer to an array of uint8_t. From inside the function I write to an outside declared array. 

 

#define OneFrameOnWire        560            // delay after Readbyte. Transmission finished

 

I need 560 us delay. I must wait the transmission off the full byte on wire before I can access the DS2482 Dataregister.

The other time is caused by TWI communication between microcontroller & DS2482

 

 

Senior Electric Engineer
--------------------------
I use Atmega128, AS7, GNU GCC, Atmel ICE

Last Edited: Mon. Apr 12, 2021 - 03:38 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Good morning  amorawala,

 

is all ok? Is Your problem solved?

 

In the meantime I've downloaded ds18b20 lib 0x02 (c) Davide Gironi, 2012. 

Made a new project on Atmega128a. With a few changes it works. 

 

1. In the ds18b20.c file is a missing declaration for F_CPU.

*/

#ifndef F_CPU
    #define F_CPU 7372800UL    
#endif

This must be declared before #include <util/delay.h>. 

Otherwise You get a warning.

(A thing which I do not understand. F_CPU is already declared in main.c. Seems to be a compiler problem)

 

2. Waiting for conversion ready:

    //while(!ds18b20_readbit()); //wait until conversion is complete

this takes 600ms. Unfavorable realized. Place a     _delay_ms(100);

 

3. In ds18b20_gettemp() he locks the interruptsystem with cli();

Here a complete workaround is needed. 

 

My code calling the functions:

 

    uint8_t buffer[8];

    ds18b20_reset(); //reset
    _delay_us(200);
    ds18b20_writebyte(DS18B20_CMD_READROM); //skip ROM
    for (uint8_t i = 0; i < 8; i++)
    {
        _delay_us(200);
        buffer[i] = ds18b20_readbyte();
    }

 

The timing on 1wire lock very well. I am satified. 

 

Summary

1 Wire on a Port pin seems to be going well.

But I stay with the system with DS2482. The benefits outweigh. 

 

regards

EllenR

 

 

 

 

Attachment(s): 

Senior Electric Engineer
--------------------------
I use Atmega128, AS7, GNU GCC, Atmel ICE

Last Edited: Tue. Apr 13, 2021 - 12:51 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

EllenR wrote:

 

1. In the ds18b20.c file is a missing declaration for F_CPU.

*/

#ifndef F_CPU
    #define F_CPU 7372800UL    
#endif

This must be declared before #include <util/delay.h>. 

Otherwise You get a warning.

(A thing which I do not understand. F_CPU is already declared in main.c. Seems to be a compiler problem)

 

 

avr-gcc would not pass any defines from main.c to any other ".c" file , that's what ".h" files are for.

 

The preferred way (for me) to define F_CPU, would be to pass it as an argument to avr-gcc in the makefile.

And just set (define) it in the makefile , along w. mcu type etc ..., that way all files will get that definition passed by avr-gcc.

 

Else you could define it in a ".h" file and include that in all ".c" files that needs that define.

 

/Bingo

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hello EllenR.

1. The F_CPU is defined in the compiler in Atmel Studio 7. 

2. What do you mean by "Unfavorable realized "? Is this a very long time? Is it not proper that the DS itself gives out a signal that the conversion is complete?

3. I will check out your code and edit as necessary.

 

I placed a 400us delay after the ds18b20_writebyte() function. Also placed a 200us delay between ds18b20_readbyte() functions.

This made my DSO traces very easy to read. 

And I actually saw the problem. The DSO shows 8 bytes being read, but displays only 5 bytes. This is because the later bytes are 0x00, which terminates the UART_SendString() function. That is why I cannot read all 8 bytes in the terminal. 

So now I have to work around how to send all bytes to the terminal using the uart, including any 00 bytes.  

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

MrKendo wrote:

This may be a separate issue, but your use of

hex2a(uint8_t*s, char*buff)

is strange.

s is pointer to array of uint8_t (with 8 or 9 entries by the looks of it), and you loop until you get a zero value in s.

What is to say one of those 8 or 9 entries won't be 0?

Looping until you get a zero is perfectly normal when the array represents a null terminated string, as you end up with in buff, but not when it is array of arbitrary data.

It would be more normal to pass in a length to say how many entries from s are to be processed.

Or just convert one uint8_t at a time.

Yes MrKendo. This is the problem. After placing proper delay loops between reading bytes, I could see that all 8 bytes were being read by the ds18b20_readbyte() function. I could also see two bytes as 0x00 on the DSO. So, the homegrown function hex2a() was terminating the conversions here, and hence all bytes were not being converted, so not sent on the usart.

As Clawson also pointed out I will try to use the STDIO standard functions to eliminate these homegrown functions. 

How do I pass the length of the array of data? Do I use strlen(), or send a specific 'number-of-bytes' as argument? This array size is variable since I use it for multiple data.

Thanks. 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

amorawala wrote:
How do I pass the length of the array of data?

You need to keep account of how much data is in the array; eg, you said above that it is 8 bytes.

 

Do I use strlen()

No:  that's another C string function - so it will take the first NUL byte as the end of the string!

 

or send a specific 'number-of-bytes' as argument?

Yes.

 

This array size is variable since I use it for multiple data.

See above - you need to keep  a count (or used a fixed constant, if known) according to each case.

 

Top Tips:

  1. How to properly post source code - see: https://www.avrfreaks.net/comment... - also how to properly include images/pictures
  2. "Garbage" characters on a serial terminal are (almost?) invariably due to wrong baud rate - see: https://learn.sparkfun.com/tutorials/serial-communication
  3. Wrong baud rate is usually due to not running at the speed you thought; check by blinking a LED to see if you get the speed you expected
  4. Difference between a crystal, and a crystal oscillatorhttps://www.avrfreaks.net/comment...
  5. When your question is resolved, mark the solution: https://www.avrfreaks.net/comment...
  6. Beginner's "Getting Started" tips: https://www.avrfreaks.net/comment...
Last Edited: Thu. Apr 15, 2021 - 08:19 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

amorawala wrote:
2. What do you mean by "Unfavorable realized "? Is this a very long time?

 

Hello amorawala,

 

Yes indeed, it's the long time. 

Our wonderful microcontrollers are able to execute 1Mips/1Mhz Clock rate. 

Gironis code getTemp takes 600ms with locked interrupt system. There is nothing else you can do in this time.

Better divide this into 2 sessions: 

1. Start Temp Convert. Do somthing other now

2. Check Conversion ready. In the case Yes read temperature, otherwise to somthing other. 

 

The ds18b20 is not able to send an interrupt signal. You can only readout this state.

 

regards 

EllenR

 

 

Senior Electric Engineer
--------------------------
I use Atmega128, AS7, GNU GCC, Atmel ICE

Last Edited: Thu. Apr 15, 2021 - 12:13 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Bingo600 wrote:
And just set (define) it in the makefile

 

Thanks for the hint

I do so

 

 

Senior Electric Engineer
--------------------------
I use Atmega128, AS7, GNU GCC, Atmel ICE

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Bingo600 wrote:
avr-gcc would not pass any defines from main.c to any other ".c" file , that's what ".h" files are for.

just to note that this is not specific to avr-gcc or, indeed, any other GCC - this is standard C behaviour.

 

The preferred way (for me) to define F_CPU, would be to pass it as an argument to avr-gcc in the makefile.

Or, if you're using an IDE, in the IDE's Project settings

 

See: https://www.avrfreaks.net/commen...

 

Illustrated: https://www.avrfreaks.net/commen...

 

 that way all files will get that definition passed by avr-gcc.

Absolutely.

 

Top Tips:

  1. How to properly post source code - see: https://www.avrfreaks.net/comment... - also how to properly include images/pictures
  2. "Garbage" characters on a serial terminal are (almost?) invariably due to wrong baud rate - see: https://learn.sparkfun.com/tutorials/serial-communication
  3. Wrong baud rate is usually due to not running at the speed you thought; check by blinking a LED to see if you get the speed you expected
  4. Difference between a crystal, and a crystal oscillatorhttps://www.avrfreaks.net/comment...
  5. When your question is resolved, mark the solution: https://www.avrfreaks.net/comment...
  6. Beginner's "Getting Started" tips: https://www.avrfreaks.net/comment...
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Fine Awneil. to keep things simple, I will send a fixed number of bytes through a counter. 

This reply has been marked as the solution. 
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

awneil wrote:

amorawala wrote:
How do I pass the length of the array of data?

You need to keep account of how much data is in the array; eg, you said above that it is 8 bytes.

 

See above - you need to keep  a count (or used a fixed constant, if known) according to each case.

 

I passed a fixed number of bytes to convert and this solved the problem. 

Thanks to all who assisted me. I will also look into the best practices such as using STDIO functions and more that you all sent me during this discussion.

 

Thanks a lot to all. 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I would also suggest that you look into getting & using a  debugger: then, among other things, you could directly examine values within the AVR itself - without having to worry about how to get them out to a terminal.

 

The ATmega328 XPlained Mini board has a built-in debugger, and is only ~ $10

 

https://www.avrfreaks.net/forum/xplained-mini-mega328pb

 

 

Top Tips:

  1. How to properly post source code - see: https://www.avrfreaks.net/comment... - also how to properly include images/pictures
  2. "Garbage" characters on a serial terminal are (almost?) invariably due to wrong baud rate - see: https://learn.sparkfun.com/tutorials/serial-communication
  3. Wrong baud rate is usually due to not running at the speed you thought; check by blinking a LED to see if you get the speed you expected
  4. Difference between a crystal, and a crystal oscillatorhttps://www.avrfreaks.net/comment...
  5. When your question is resolved, mark the solution: https://www.avrfreaks.net/comment...
  6. Beginner's "Getting Started" tips: https://www.avrfreaks.net/comment...