patch: add error code handling of temperatures

[Andersama]

tentative patch for handling issue #236

• adds unit types to force handling of different units with respect to each other
• implicit conversions to floats only for result types from existing functions
• basic error codes added as an enum, edit as needed

Please test that this compiles on hardware before commiting* I don't have an arduino laying around anymore.

Just looked back at the underlying code for the basic errors I added, the originals all appear to be from a bitmask, could be considerably faster to just pass along the error state from what was read. EG: take

        if (scratchPad[TEMP_LSB] & 1) { // Fault Detected
            if (scratchPad[HIGH_ALARM_TEMP] & 1) {
                //Serial.println("open detected");
                r.reading.raw = DEVICE_FAULT_OPEN_RAW;
                r.error_code = DallasTemperature::device_error_code::device_fault_open;
                return r; //return DEVICE_FAULT_OPEN_RAW;
            }
            else if (scratchPad[HIGH_ALARM_TEMP] >> 1 & 1) {
                //Serial.println("short to ground detected");
                r.reading.raw = DEVICE_FAULT_SHORTGND_RAW;
                r.error_code = DallasTemperature::device_error_code::device_fault_shortgnd;
                return r;
            }
            else if (scratchPad[HIGH_ALARM_TEMP] >> 2 & 1) {
                //Serial.println("short to Vdd detected");
                r.reading.raw = DEVICE_FAULT_SHORTVDD_RAW;
                r.error_code = DallasTemperature::device_error_code::device_fault_shortvdd;
                return r;
            }
            else {
                // We don't know why there's a fault, exit with disconnect value
                r.reading.raw = DEVICE_DISCONNECTED_RAW;
                r.error_code = DallasTemperature::device_error_code::device_disconnected;
                return r;
            }
        }

and turn it into something like:

        if (scratchPad[TEMP_LSB] & 1) { // Fault Detected
                    r.reading.raw = (scratchPad[HIGH_ALARM_TEMP] & 0x7) | 0x8; // treat 0x8 as "fault detected" or "disconnected"
                r.error_code = DallasTemperature::device_error_code::device_fault_open;
                return r;
        }

[Andersama]

Some example usage:

bool address_ok = sensors.getAddress(deviceAddress, 0);
if (!address_ok)
   return;

DallasTemperature::request_t request = sensors.requestTemperatures();
DallasTemperature::celsius_result_t temp_reading= sensors.getTempC(deviceAddress);
// consider utility functions for testing for specific error flags / codes
if (temp_reading.error_code & DallasTemperature::device_error_code::device_fault_open) {
   //display error for shorted ground
}
if (temp_reading.error_code & DallasTemperature::device_error_code::device_fault_shortgnd) {
   //display error for shorted ground
}
if (temp_reading.error_code & DallasTemperature::device_error_code::device_fault_shortvdd) {
   //display error for shorted vdd
}
// etc...for other errors
if (temp_reading.error_code != DallasTemperature::device_error_code::device_connected) {
   return;
}

float f = temp_reading.value.celcius; // do something with the temperature
//float f = temp_reading; // for backwards compatibility

[LokiMetaSmith]

If I understand this correctly, applying this patch would require refactoring existing usage of the library. I don't think that's acceptable as that would require firmware changes to every project that uses this library. --edit I see how it works now. I'm testing this in a project that needs error reporting. will report back results

[Andersama]

You may want to add the other edit for simplifying the flags, should in theory cut the number of jumps in the resulting code. Not sure if I made a patch for that in my own fork.

[ForrestErickson]

Hello @Andersama

I am trying your example code. I get an error compiling at the line float f = temp_reading.celcius; // do something with the temperature

Compiler error

C:\Users\Public\Downloads\Arduino\PubInv\DallasTemprature\Arduino-Temperature-Control-Library\Arduino-Temperature-Control-Library.ino: In function 'void isError_AndPrintTemperature(uint8_t*)':
Arduino-Temperature-Control-Library:200:26: error: 'struct DallasTemperature::celsius_result_t' has no member named 'celcius'
   float f = temp_reading.celcius; // do something with the temperature
                          ^
Multiple libraries were found for "OneWire.h"
 Used: C:\Users\Admin\Documents\Arduino\libraries\OneWire
 Not used: C:\Users\Admin\Documents\Arduino\libraries\MAX31850_OneWire
exit status 1
'struct DallasTemperature::celsius_result_t' has no member named 'celcius'

I am attaching a zip of my sketch which produced this error on an Arduino Due Arduino-Temperature-Control-LibraryForAndersama.zip

Full Example Please

Could you provide a fully functional example which compiles? I do not read C++ well enough to trouble shoot this error.

Thanks.

[Andersama]

@ForrestErickson well glad you decided to test it first. That error's saying that the type doesn't contain a member called "celsius" which, on second viewing, is true. Badly written example on my part try value.celsius.

[Andersama]

@LokiMetaSmith yeah, from what I remember there was a discussion about magic numbers somewhere, so this was my suggestion instead. I kept the original magic values in the commit so this should be backwards compatible. I was aiming though to remove the nested if/else if chain.

[milesburton]

Like you @Andersama I don't have an Arduino to hand so I'm hesitant to merge. @RobTillaart any thoughts?

[RobTillaart]

Had a quick look and besides error codes it introduces a few new types. I understand that this allows compile time checking, which is good.

However what worries me is that it might make existing projects more complex for the average Arduino user.

Furthermore does this extra type checking help the novice Arduino user? I know arguments for both sides.

So I need to review the details in more depth and check what happens with existing code too. I do not have time to dive into these tests on short term.

[RobTillaart]

@milesburton I see the logs of the build are removed (auto xleanup). Can you redo the build ci test, as that would show if there are more warnings or so.

[milesburton]

Annoyingly that doesn't appear to be as straight forward as I'd like. Let me pull the change and push a dummy commit. Hopefully that'll persuade github to rerun the jobs

[Andersama]

Well whatever exactly I was thinking at the time's a bit lost to me at this point, it's been a year, but I do remember trying to make it compatible abi wise as possible.

[ForrestErickson]

FYI, I am traveling now and will not be back to my lab and hardware for some weeks. I have an Arduino DUE system with three MAX31850 on which I can test once I get caught up.