bonnyr / wokwi-ds1820-custom-chip

MIT License
2 stars 3 forks source link

ds18B20 negative temperature issue #17

Closed dpticar closed 1 year ago

dpticar commented 1 year ago

Hi,

When using "family_code": "40" in attribute, thats ds18B20, negative temperatures are wrong. For example -55 C should be 0xFC90 but it's 0xFB70.

Also family_code attribute can not be set in HEX, for example "family_code": "0x28" is not working.

Here is link to simulation : https://wokwi.com/projects/359211982602037249

bonnyr commented 1 year ago

Thanks for your feedback!

A couple of notes/questions:

  1. regarding the attribute values - I do not control the manner in which the values can be provided in diagram.json so unfortunately the syntax does not support things like 0x28. I will update the document to reflect that.
  2. Regarding the negative reading values. I am not sure how this code is interpreting the temperature, but I suspect that sign extension is causing issues in the way that you are computing this value. Can you tell me more about the manner in which this is calculated?

Having said that (for 2.) I think that there is also a bug in the way the temperature is stored in the scratchpad - I am looking at the code now :)

dpticar commented 1 year ago

Hi,

Well I'm calculating temperature like it's stated in ds18b20.pdf by using 16-bit signed integer (two's complement)

In ds18b20.pdf on page 6, there are table with 10 example values for temperatures.

Code below is doing temperature to MSB+LSB values. In simulation I'm doing opposite, taking MSB + LSB and putting it in int16_t (that's temperature*16), than putting that in float and dividing by 16 to get real temp.

--- Temperature -> MSB+LSB

float temp=-50;
int16_t intTemp=(temp*16);
printf("MSB+LSB = %x\n",intTemp);

uint8_t LSB = intTemp&0xFF;
uint8_t MSB = intTemp>>8;

printf("LSB = %x\n",LSB);
printf("MSB = %x\n",MSB);
dpticar commented 1 year ago

Hi,

this is not affecting simulation but for DS18B20 (0x28 device id) BYTE-7 in SCRATCHPAD should be 0x10 and BYTE-5 should be 0xFF. (ds18b20.pdf, page 8)

bonnyr commented 1 year ago

Hi,

this is not affecting simulation but for DS18B20 (0x28 device id) BYTE-7 in SCRATCHPAD should be 0x10 and BYTE-5 should be 0xFF. (ds18b20.pdf, page 8)

Thanks for this - I did not bother with those registers because I've never used them :)

I also found out the issue with the conversion - it was a stupid and over complicated way to store the temperature in the scratch pad that got carried over from another device - clearly not needed for the DS18B20.

I have also updated the README to state that the family code should be provided as an integer value

I will upload a new version soon.

You could either use this new version with a custom chip or wait for Wokwi to ratify this (I will raise a PR that will point to the new version.

Thank you very much for trying and helping me improve this chip.

bonnyr commented 1 year ago

@dpticar can you please test this out by using a custom chip with version 0.4.4?

dpticar commented 1 year ago

Hi,

Everything looks OK. I have not tested alarm as I'm not using it.

Also I think that ds18B20 is in 12bit resolution by default !

Probably some people does not know to set configuration bits to use 12 bit resolution, so they will think that something is not working.

So I'm suggesting to set 12bit resolution by default as it's on real ds18b20.

bonnyr commented 1 year ago

@dpticar

You're right about the default resolution value, I will need to change the POR value for the register. However note that since there's no EEPROM, it all has to behave like the chip has not been programmed.

If you'd like to contribute to this project and provide a PR that would be awesome too :)