Skip to content

Commit 6c26acd

Browse files
committed
fix(skill): Improve worked example for v1 add sensor
1 parent b8a0f02 commit 6c26acd

1 file changed

Lines changed: 166 additions & 13 deletions

File tree

  • .agents/skills/add_sensor_component_v1

.agents/skills/add_sensor_component_v1/SKILL.md

Lines changed: 166 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,38 @@ methods in the base driver class:
8282

8383
Temperature sensors almost always include both `ambient-temp` and `ambient-temp-fahrenheit`.
8484

85+
**Important:** Fahrenheit conversions (`getEventAmbientTempF`, `getEventObjectTempF`) are already
86+
implemented in the base class — they call the Celsius method and convert. Drivers only need to
87+
implement the Celsius version (`getEventAmbientTemp`, `getEventObjectTemp`). Never implement
88+
the Fahrenheit variant in your driver.
89+
90+
Because the base class Fahrenheit method calls the Celsius method, and the calling order is not
91+
guaranteed (°F may be called before °C), each `getEvent*()` method should go through a shared
92+
read-and-cache function with a "recently read" time guard. This way, whichever method is called
93+
first does the actual I2C read and caches the result; subsequent calls within the window return
94+
cached data without hitting the bus again.
95+
96+
See `WipperSnapper_I2C_Driver_SCD30.h` for the canonical pattern:
97+
```cpp
98+
bool HasBeenReadInLastSecond() {
99+
return _lastRead != 0 && millis() - _lastRead < 1000;
100+
}
101+
bool ReadSensorData() {
102+
if (HasBeenReadInLastSecond()) return true; // use cached values
103+
// ... do actual I2C read, cache results ...
104+
_lastRead = millis();
105+
return true;
106+
}
107+
bool getEventAmbientTemp(sensors_event_t *tempEvent) {
108+
if (!ReadSensorData()) return false;
109+
*tempEvent = _cachedTemp;
110+
return true;
111+
}
112+
```
113+
114+
This is especially important for multi-reading sensors but also applies to temperature sensors
115+
where both °C and °F subcomponents are enabled.
116+
85117
---
86118
87119
## Step 1 — Create the Driver Header
@@ -512,20 +544,141 @@ Two separate PRs are needed:
512544

513545
## Worked Example: TMP119
514546

515-
The TMP119 is a high-accuracy temperature sensor from Texas Instruments. It's a variant of the
516-
TMP117 with a different chip ID (0x2117 vs 0x0117).
547+
The TMP119 is a high-accuracy temperature sensor from Texas Instruments, a variant of the TMP117
548+
with a different chip ID (0x2117 vs 0x0117).
549+
550+
### Step 0 — Research
517551

518-
- **Library:** `Adafruit_TMP117` (contains both `Adafruit_TMP117` and `Adafruit_TMP119` classes)
519-
- **API:** `begin(uint8_t addr, TwoWire *wire)`, `getEvent(sensors_event_t *)`
520-
- **I2C addresses:** 0x48, 0x49, 0x4A, 0x4B
521-
- **Measures:** Temperature (ambient-temp, ambient-temp-fahrenheit)
552+
- **Search:** `gh search repos "Adafruit TMP119" --owner adafruit` finds only the PCB repo, no
553+
standalone Arduino library. But checking the `Adafruit_TMP117` library reveals
554+
`Adafruit_TMP119.h` and `.cpp` inside it — the TMP119 class inherits from TMP117.
555+
- **Library:** `Adafruit_TMP117` (contains `Adafruit_TMP119` class)
556+
- **I2C addresses:** 0x48, 0x49, 0x4A, 0x4B (same as TMP117, datasheet Table 7-1)
557+
- **Measures:** Temperature only → subcomponents: `ambient-temp`, `ambient-temp-fahrenheit`
522558
- **Closest driver:** `WipperSnapper_I2C_Driver_TMP117.h`
523-
- **platformio.ini:** Already has `adafruit/Adafruit TMP117` — no change needed
524559

525-
### Files changed:
560+
### Step 1 — Read the example, then the library source
561+
562+
**Example** (`examples/TMP119_basic_test/TMP119_basic_test.ino`):
563+
```cpp
564+
Adafruit_TMP119 tmp11x;
565+
tmp11x.begin(); // default addr 0x48, Wire
566+
while (!tmp11x.dataReady()) delay(10); // polls data-ready flag
567+
tmp11x.getEvent(&temp); // fills sensors_event_t
568+
```
569+
570+
**Library source** (`Adafruit_TMP117.h` / `Adafruit_TMP119.cpp`):
571+
- `_init()` calls `reset()` which restores factory defaults:
572+
- Continuous conversion mode (`TMP117_MODE_CONTINUOUS`)
573+
- 8x averaging (`TMP117_AVERAGE_8X`)
574+
- 1000ms conversion delay (`TMP117_DELAY_1000_MS`)
575+
- `getEvent()` internally calls `waitForData()` which blocks until `dataReady()` is true
576+
- `begin(addr, wire)` — address first, wire second
577+
578+
**Decisions:**
579+
- The library's `getEvent()` handles data-ready blocking internally, so no `fastTick()`.
580+
- Explicitly set mode and averaging in `begin()` to pin the defaults.
581+
- Only implement `getEventAmbientTemp` (Celsius) — the base class handles °F conversion.
582+
- Since the calling order of °C and °F methods is not guaranteed, use a shared read-and-cache
583+
pattern with a time guard so only the first call per cycle hits the I2C bus:
584+
585+
```cpp
586+
protected:
587+
Adafruit_TMP119 *_tmp119;
588+
sensors_event_t _cachedTemp = {0};
589+
unsigned long _lastRead = 0;
590+
591+
bool _readSensor() {
592+
if (_lastRead != 0 && millis() - _lastRead < 1000)
593+
return true; // recently read, use cached value
594+
if (!_tmp119->getEvent(&_cachedTemp))
595+
return false;
596+
_lastRead = millis();
597+
return true;
598+
}
599+
600+
public:
601+
bool begin() {
602+
_tmp119 = new Adafruit_TMP119();
603+
if (!_tmp119->begin((uint8_t)_sensorAddress, _i2c))
604+
return false;
605+
// Pin defaults explicitly — library reset() sets these, but we don't
606+
// want a future library change to silently alter WipperSnapper behavior
607+
_tmp119->setMeasurementMode(TMP117_MODE_CONTINUOUS);
608+
_tmp119->setAveragedSampleCount(TMP117_AVERAGE_8X);
609+
return true;
610+
}
611+
612+
bool getEventAmbientTemp(sensors_event_t *tempEvent) {
613+
if (!_readSensor())
614+
return false;
615+
*tempEvent = _cachedTemp;
616+
return true;
617+
}
618+
// getEventAmbientTempF is inherited — it calls getEventAmbientTemp and converts
619+
```
620+
621+
### Step 2 — Register in WipperSnapper_I2C.h
622+
623+
```cpp
624+
// After TMP117 include (alphabetical)
625+
#include "drivers/WipperSnapper_I2C_Driver_TMP119.h"
626+
627+
// In private section, after _tmp117
628+
WipperSnapper_I2C_Driver_TMP119 *_tmp119 = nullptr;
629+
```
630+
631+
### Step 3 — Init block in WipperSnapper_I2C.cpp
632+
633+
```cpp
634+
// After the tmp117 block, before tsl2591 (alphabetical)
635+
} else if (strcmp("tmp119", msgDeviceInitReq->i2c_device_name) == 0) {
636+
_tmp119 = new WipperSnapper_I2C_Driver_TMP119(this->_i2c, i2cAddress);
637+
if (!_tmp119->begin()) {
638+
WS_DEBUG_PRINTLN("ERROR: Failed to initialize TMP119!");
639+
_busStatusResponse =
640+
wippersnapper_i2c_v1_BusResponse_BUS_RESPONSE_DEVICE_INIT_FAIL;
641+
return false;
642+
}
643+
_tmp119->configureDriver(msgDeviceInitReq);
644+
drivers.push_back(_tmp119);
645+
WS_DEBUG_PRINTLN("TMP119 Initialized Successfully!");
646+
}
647+
```
648+
649+
### Step 4 — Library dependency
650+
651+
`platformio.ini` already has `adafruit/Adafruit TMP117` and `library.properties` already has
652+
`Adafruit TMP117` — no changes needed since TMP119 lives in that package.
653+
654+
### Step 5 — Component definition
655+
656+
`Wippersnapper_Components/components/i2c/tmp119/definition.json`:
657+
```json
658+
{
659+
"displayName": "TMP119",
660+
"vendor": "Texas Instruments",
661+
"productURL": "https://www.adafruit.com/product/6201",
662+
"documentationURL": "https://learn.adafruit.com/adafruit-tmp117-high-accuracy-i2c-temperature-monitor",
663+
"published": false,
664+
"i2cAddresses": ["0x48", "0x49", "0x4A", "0x4B"],
665+
"subcomponents": ["ambient-temp", "ambient-temp-fahrenheit"]
666+
}
667+
```
668+
669+
Simple string subcomponents are fine here — "ambient-temp" is unambiguous for a temperature-only
670+
sensor. No need for the object format.
671+
672+
Image: grab from Adafruit product API, resize to 400x300, compress.
673+
674+
### Files changed
526675

527-
1. **New:** `src/components/i2c/drivers/WipperSnapper_I2C_Driver_TMP119.h`
528-
2. **Modified:** `src/components/i2c/WipperSnapper_I2C.h` (include + pointer)
529-
3. **Modified:** `src/components/i2c/WipperSnapper_I2C.cpp` (init block)
530-
4. **New (components repo):** `components/i2c/tmp119/definition.json`
531-
5. **New (components repo):** `components/i2c/tmp119/image.jpg`
676+
| File | Action |
677+
|------|--------|
678+
| `src/components/i2c/drivers/WipperSnapper_I2C_Driver_TMP119.h` | New — driver with explicit mode/averaging config |
679+
| `src/components/i2c/WipperSnapper_I2C.h` | Modified — include + private pointer |
680+
| `src/components/i2c/WipperSnapper_I2C.cpp` | Modified — init block in `initI2CDevice()` |
681+
| `platformio.ini` | No change — TMP117 library already listed |
682+
| `library.properties` | No change — TMP117 library already listed |
683+
| `Wippersnapper_Components/components/i2c/tmp119/definition.json` | New |
684+
| `Wippersnapper_Components/components/i2c/tmp119/image.jpg` | New |

0 commit comments

Comments
 (0)