diff --git a/experimental/devices/mcp9808/mcp9808.go b/experimental/devices/mcp9808/mcp9808.go index e4c20db..cb45c81 100644 --- a/experimental/devices/mcp9808/mcp9808.go +++ b/experimental/devices/mcp9808/mcp9808.go @@ -266,8 +266,8 @@ func (d *Dev) readTemperature() (physic.Temperature, uint8, error) { // Convert to physic.Temperature 0.0625°C per bit t := physic.Temperature(tbits&0x0FFF) * 62500 * physic.MicroKelvin if tbits&0x1000 > 0 { - // Check for bit sign. - t = -t + // Check for sign bit. + t -= 256 * physic.Celsius } t += physic.ZeroCelsius return t, uint8(tbits>>8) & 0xe0, nil @@ -386,7 +386,7 @@ func alertBitsToTemperature(b uint16) physic.Temperature { b = (b >> 2) & 0x07FF t := physic.Temperature(b&0x03FF) * 250 * physic.MilliKelvin if b&0x400 > 0 { - t = -t + t -= 256 * physic.Celsius } t += physic.ZeroCelsius return t @@ -403,13 +403,14 @@ func alertTemperatureToBits(t physic.Temperature) (uint16, error) { // 0.25°C per bit. t /= 250 * physic.MilliKelvin - var bits uint16 - if t < 0 { - t = -t - bits |= 0x400 - } - bits |= uint16(t) - bits = bits << 2 + // We don't need to explicitly handle negative temperatures because both Go and the MCP9808 + // store negative values using two's complement. We can rely on Go's implementation since + // we know that the bits of a negative value are already in two's complement, implying that + // the sign bit will already be set to 1 due to the range check. We need to be sure to mask + // off the 3 most significant bits after shifting though: they will all be set to 1 if the + // value is negative. Also mask off the 2 least significant bits. While not strictly necessary, + // the MCP9808 doesn't use them. + bits := (uint16(t) << 2) & 0x1ffc return bits, nil } diff --git a/experimental/devices/mcp9808/mcp9808_test.go b/experimental/devices/mcp9808/mcp9808_test.go index fab8198..96581ca 100644 --- a/experimental/devices/mcp9808/mcp9808_test.go +++ b/experimental/devices/mcp9808/mcp9808_test.go @@ -94,7 +94,8 @@ func TestSense(t *testing.T) { { name: "-10C", tx: []i2ctest.IO{ - {Addr: 0x18, W: []byte{temperature}, R: []byte{0x10, 0xa0}}, + // 0x1f60 is -10°C in two's complement. See page 24-25 of the datasheet. + {Addr: 0x18, W: []byte{temperature}, R: []byte{0x1f, 0x60}}, }, enabled: true, want: physic.ZeroCelsius - 10*physic.Kelvin, @@ -284,7 +285,8 @@ func TestSenseTemp(t *testing.T) { { name: "-10C", tx: []i2ctest.IO{ - {Addr: 0x18, W: []byte{temperature}, R: []byte{0x10, 0xa0}}, + // 0x1f60 is -10°C in two's complement. See page 24-25 of the datasheet. + {Addr: 0x18, W: []byte{temperature}, R: []byte{0x1f, 0x60}}, }, want: physic.ZeroCelsius - 10*physic.Kelvin, err: nil, @@ -356,7 +358,9 @@ func TestSenseWithAlerts(t *testing.T) { {Addr: 0x18, W: []byte{critAlert, 0x00, 0xa0}, R: nil}, {Addr: 0x18, W: []byte{upperAlert, 0x00, 0x50}, R: nil}, {Addr: 0x18, W: []byte{lowerAlert, 0x00, 0x00}, R: nil}, - {Addr: 0x18, W: []byte{temperature}, R: []byte{0x30, 0x10}}, + // 0x3ff0 is -1°C in two's complement with the lower alert bit set (bit 13). + // See page 24-25 of the datasheet. + {Addr: 0x18, W: []byte{temperature}, R: []byte{0x3f, 0xf0}}, {Addr: 0x18, W: []byte{lowerAlert}, R: []byte{0x00, 0x00}}, }, err: nil, @@ -487,7 +491,9 @@ func TestSenseWithAlerts(t *testing.T) { {Addr: 0x18, W: []byte{critAlert, 0x00, 0xa0}, R: nil}, {Addr: 0x18, W: []byte{upperAlert, 0x00, 0x50}, R: nil}, {Addr: 0x18, W: []byte{lowerAlert, 0x00, 0x00}, R: nil}, - {Addr: 0x18, W: []byte{temperature}, R: []byte{0x30, 0x10}}, + // 0x3ff0 is -1°C in two's complement with the lower alert bit set (bit 13). + // See page 24-25 of the datasheet. + {Addr: 0x18, W: []byte{temperature}, R: []byte{0x3f, 0xf0}}, }, err: errReadLowerAlert, }, @@ -897,7 +903,9 @@ func Test_alertBitsToTemperature(t *testing.T) { }{ {"0°C", 0x0000, physic.ZeroCelsius}, {"0.25°C", 0x0004, physic.ZeroCelsius + 250*physic.MilliKelvin}, - {"-0.25°C", 0x1004, physic.ZeroCelsius - 250*physic.MilliKelvin}, + // Negative values are in two's complement. See page 22 of the datasheet. + {"-0.25°C", 0x1ffc, physic.ZeroCelsius - 250*physic.MilliKelvin}, + {"-39.75°C", 0x1d84, physic.ZeroCelsius - 39750*physic.MilliKelvin}, } for _, tt := range tests { @@ -907,7 +915,7 @@ func Test_alertBitsToTemperature(t *testing.T) { } } -func Test_temperatureToAlertBits(t *testing.T) { +func Test_alertTemperatureToBits(t *testing.T) { succeeds := []struct { name string alert physic.Temperature @@ -915,9 +923,10 @@ func Test_temperatureToAlertBits(t *testing.T) { }{ {"0°C", physic.ZeroCelsius, 0x0000}, {"0.25°C", physic.ZeroCelsius + 250*physic.MilliKelvin, 0x0004}, - {"-0.25°C", physic.ZeroCelsius - 250*physic.MilliKelvin, 0x1004}, {"124.75°C", physic.ZeroCelsius + 124750*physic.MilliKelvin, 0x07cc}, - {"-39.75°C", physic.ZeroCelsius - 39750*physic.MilliKelvin, 0x127c}, + // Negative values are in two's complement. See page 22 of the datasheet. + {"-0.25°C", physic.ZeroCelsius - 250*physic.MilliKelvin, 0x1ffc}, + {"-39.75°C", physic.ZeroCelsius - 39750*physic.MilliKelvin, 0x1d84}, } fails := []struct { @@ -932,16 +941,16 @@ func Test_temperatureToAlertBits(t *testing.T) { for _, tt := range succeeds { got, err := alertTemperatureToBits(tt.alert) if got != tt.want && err == nil { - t.Errorf("alertBitsToTemperature(%s) = %x, want %x", tt.name, got, tt.want) + t.Errorf("alertTemperatureToBits(%s) = %x, want %x", tt.name, got, tt.want) } if err != nil { - t.Errorf("alertBitsToTemperature(%s) got unexpected error: %v", tt.name, err) + t.Errorf("alertTemperatureToBits(%s) got unexpected error: %v", tt.name, err) } } for _, tt := range fails { if _, err := alertTemperatureToBits(tt.alert); err == nil { - t.Errorf("alertBitsToTemperature(%s) expected error %v", tt.name, tt.want) + t.Errorf("alertTemperatureToBits(%s) expected error %v", tt.name, tt.want) } } }