Feature/p3t1755 by aolgeirson · Pull Request #379 · Northeastern-Electric-Racing/Embedded-Base

aolgeirson · 2026-03-20T18:18:18Z

Changes

Fixes for the p3t1755 onboard temp sensor for Shepherd

Copilot

Pull request overview

Fixes the P3T1755 onboard temperature sensor driver for the Shepherd platform, primarily by adjusting I2C addressing and updating temperature conversion logic.

Changes:

Left-shift the I2C device address during p3t1755_init() to match the project’s I2C driver expectations.
Introduce a new raw-to-celsius conversion helper and update p3t1755_read_temperature() to use it.
Apply formatting/cleanup updates across the driver and header.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 7 comments.

File	Description
`general/src/p3t1755.c`	Updates address handling and rewrites temperature read/convert path (plus formatting).
`general/include/p3t1755.h`	Adds a (problematic) declaration for the new conversion helper and reformats defines/types.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-20T18:22:29Z

general/src/p3t1755.c

+  *temp_c = p3t1755_RAW_TO_CELSIUS(
+      uint8_to_uint16(high_temp_reg[0], high_temp_reg[1]));
+  return status;


Temperature conversion logic is now different between p3t1755_read_temperature() (custom helper + bit shifting) and this threshold read, which still uses p3t1755_RAW_TO_CELSIUS(uint8_to_uint16(...)) with no shifting. If the registers are left-justified (common with 0.0625°C parts), threshold reads will disagree with the current temperature read. Recommend decoding T_HIGH using the same raw->°C path as the TEMPERATURE register (and similarly for T_LOW / threshold writes).

Copilot · 2026-03-20T18:22:30Z

general/src/p3t1755.c

+  *temp_c =
+      p3t1755_RAW_TO_CELSIUS(uint8_to_uint16(low_temp_reg[0], low_temp_reg[1]));


Same issue as T_HIGH: this uses p3t1755_RAW_TO_CELSIUS(uint8_to_uint16(...)) without applying the nibble shift/sign-extension logic introduced for the TEMPERATURE register. Consider sharing a single decode routine so T_LOW, T_HIGH, and TEMPERATURE all interpret the sensor’s register format consistently.

Copilot · 2026-03-20T18:22:30Z

general/src/p3t1755.c

-int p3t1755_read_temperature(p3t1755_t *p3t, float *temp_c)
-{
-	uint8_t temp_reg[2];
+inline float p3t1755_raw_to_celsius(uint16_t raw) {


p3t1755_raw_to_celsius() is defined as inline with external linkage in the .c file, while the header declares it static inline (and without a definition). C inline linkage rules can easily produce missing/duplicate symbols depending on compiler flags. Prefer making this helper static/static inline in the .c file (and removing it from the public header), or provide a single non-inline extern declaration in the header with one .c definition.

Suggested change

inline float p3t1755_raw_to_celsius(uint16_t raw) {

static inline float p3t1755_raw_to_celsius(uint16_t raw) {

Copilot · 2026-03-20T18:22:30Z

general/src/p3t1755.c

+  *temp_c = p3t1755_raw_to_celsius(
+      uint8_to_uint16(temp_reg[0] >> 4, temp_reg[1] >> 4));


Temperature register bytes are combined incorrectly: uint8_to_uint16(temp_reg[0] >> 4, temp_reg[1] >> 4) drops most of the MSB and will produce a near-zero raw value (and the sign bit check in p3t1755_raw_to_celsius() will never trigger). Reconstruct the 16-bit register value from the unshifted bytes first, then right-shift by 4 (per the sensor’s 0.0625°C format) before converting to °C.

Suggested change

*temp_c = p3t1755_raw_to_celsius(

uint8_to_uint16(temp_reg[0] >> 4, temp_reg[1] >> 4));

uint16_t raw = uint8_to_uint16(temp_reg[0], temp_reg[1]);

raw >>= 4;

*temp_c = p3t1755_raw_to_celsius(raw);

Copilot · 2026-03-20T18:22:31Z

general/src/p3t1755.c

+  PRINTLN_INFO("raw temp: %d", raw);
+  if (raw & 1 << 11) { // Check if sign bit is set |-> t<0
+    return -(raw * p3t1755_TEMP_RESOLUTION);
+  } else
+    return (raw * p3t1755_TEMP_RESOLUTION);


p3t1755_raw_to_celsius() does not correctly handle negative temperatures. Simply negating raw when a sign bit is set is not equivalent to decoding the 12-bit two’s-complement value; you need to sign-extend the 12-bit quantity (after shifting) into an int16_t and then apply the 0.0625°C resolution.

Suggested change

PRINTLN_INFO("raw temp: %d", raw);

if (raw & 1 << 11) { // Check if sign bit is set |-> t<0

return -(raw * p3t1755_TEMP_RESOLUTION);

} else

return (raw * p3t1755_TEMP_RESOLUTION);

/* raw contains a 12-bit two's-complement temperature value */

uint16_t raw12 = raw & 0x0FFF; /* keep only the lower 12 bits */

int16_t temp_counts;

if (raw12 & 0x0800) {

/* negative value: sign-extend 12-bit to 16-bit */

temp_counts = (int16_t)(raw12 | 0xF000);

} else {

/* positive value */

temp_counts = (int16_t)raw12;

}

PRINTLN_INFO("raw temp counts: %d", temp_counts);

return (float)temp_counts * p3t1755_TEMP_RESOLUTION;

Copilot · 2026-03-20T18:22:31Z

general/src/p3t1755.c

+inline float p3t1755_raw_to_celsius(uint16_t raw) {
+  PRINTLN_INFO("raw temp: %d", raw);


This helper prints every raw temperature conversion (PRINTLN_INFO("raw temp: ...")). That will spam logs in normal operation and can significantly increase UART/RTOS overhead on a periodic sensor read. Consider removing this, or gating it behind a compile-time debug flag / higher log level used only during bring-up.

Suggested change

inline float p3t1755_raw_to_celsius(uint16_t raw) {

PRINTLN_INFO("raw temp: %d", raw);

inline float p3t1755_raw_to_celsius(uint16_t raw) {

#ifdef P3T1755_DEBUG

PRINTLN_INFO("raw temp: %d", raw);

#endif

Copilot · 2026-03-20T18:22:31Z

general/include/p3t1755.h

+  ReadPtr read;
 } p3t1755_t;

+static inline float p3t1755_raw_to_celsius(uint16_t raw);


Header declares static inline float p3t1755_raw_to_celsius(uint16_t raw); but does not provide a definition. A static inline function should be defined in the header (so each translation unit can inline it), or it should be declared as a normal extern function and defined once in the .c file. As-is, any use from another .c file will fail to link/compile depending on compiler settings.

Suggested change

static inline float p3t1755_raw_to_celsius(uint16_t raw);

static inline float p3t1755_raw_to_celsius(uint16_t raw) {

return ((int16_t)raw) * p3t1755_TEMP_RESOLUTION;

}

aolgeirson added 3 commits March 13, 2026 17:54

fixed p3t bitshift

c576493

progress on raw to temp

6dbd277

int |-> float fixed p3t readings

0b337d4

Copilot AI review requested due to automatic review settings March 20, 2026 18:18

Copilot started reviewing on behalf of aolgeirson March 20, 2026 18:18 View session

Copilot AI reviewed Mar 20, 2026

View reviewed changes

caiodasilva2005 added 3 commits March 20, 2026 15:03

Merge branch 'main' into feature/p3t1755

100e305

fix header

f6b9631

format

df9cd91

caiodasilva2005 merged commit e11e8ee into main Mar 20, 2026
2 checks passed

caiodasilva2005 deleted the feature/p3t1755 branch March 20, 2026 19:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/p3t1755#379

Feature/p3t1755#379
caiodasilva2005 merged 6 commits intomainfrom
feature/p3t1755

aolgeirson commented Mar 20, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 20, 2026

Uh oh!

Copilot AI Mar 20, 2026

Uh oh!

Copilot AI Mar 20, 2026

Uh oh!

Copilot AI Mar 20, 2026

Uh oh!

Copilot AI Mar 20, 2026

Uh oh!

Copilot AI Mar 20, 2026

Uh oh!

Copilot AI Mar 20, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		*temp_c =
		p3t1755_RAW_TO_CELSIUS(uint8_to_uint16(low_temp_reg[0], low_temp_reg[1]));

	inline float p3t1755_raw_to_celsius(uint16_t raw) {
	static inline float p3t1755_raw_to_celsius(uint16_t raw) {

		*temp_c = p3t1755_raw_to_celsius(
		uint8_to_uint16(temp_reg[0] >> 4, temp_reg[1] >> 4));

-  *temp_c = p3t1755_raw_to_celsius(
-      uint8_to_uint16(temp_reg[0] >> 4, temp_reg[1] >> 4));
+  uint16_t raw = uint8_to_uint16(temp_reg[0], temp_reg[1]);
+  raw >>= 4;
+  *temp_c = p3t1755_raw_to_celsius(raw);

-  PRINTLN_INFO("raw temp: %d", raw);
-  if (raw & 1 << 11) { // Check if sign bit is set |-> t<0
-    return -(raw * p3t1755_TEMP_RESOLUTION);
-  } else
-    return (raw * p3t1755_TEMP_RESOLUTION);
+  /* raw contains a 12-bit two's-complement temperature value */
+  uint16_t raw12 = raw & 0x0FFF; /* keep only the lower 12 bits */
+  int16_t temp_counts;
+  if (raw12 & 0x0800) {
+    /* negative value: sign-extend 12-bit to 16-bit */
+    temp_counts = (int16_t)(raw12 | 0xF000);
+  } else {
+    /* positive value */
+    temp_counts = (int16_t)raw12;
+  }
+  PRINTLN_INFO("raw temp counts: %d", temp_counts);
+  return (float)temp_counts * p3t1755_TEMP_RESOLUTION;

		inline float p3t1755_raw_to_celsius(uint16_t raw) {
		PRINTLN_INFO("raw temp: %d", raw);

-static inline float p3t1755_raw_to_celsius(uint16_t raw);
+static inline float p3t1755_raw_to_celsius(uint16_t raw) {
+  return ((int16_t)raw) * p3t1755_TEMP_RESOLUTION;
+}

Conversation

aolgeirson commented Mar 20, 2026

Changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants