Skip to content

Commit 15bb074

Browse files
committed
fix(skill): Update I2C sensor component guide with naming conventions and improved subcomponent details
1 parent 6c26acd commit 15bb074

1 file changed

Lines changed: 82 additions & 67 deletions

File tree

  • .agents/skills/add_sensor_component_v1

.agents/skills/add_sensor_component_v1/SKILL.md

Lines changed: 82 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,16 @@ The user will supply a sensor name (e.g. "TMP119"). Your job is to research the
2121
the right Adafruit Arduino library, find the closest existing driver as a template, and then walk
2222
through every file change needed.
2323

24+
### Naming convention
25+
26+
Two naming styles are used throughout — keep them consistent:
27+
28+
- **PascalCase** for C++ identifiers: class name `WipperSnapper_I2C_Driver_TMP119`, file name
29+
`WipperSnapper_I2C_Driver_TMP119.h`, member pointer `_tmp119`, include guard `_TMP119_H`
30+
- **lowercase** for the component folder name and the `strcmp` device name string: `tmp119`
31+
32+
Decide on the canonical name early (Step 0) and use it everywhere.
33+
2434
> **Proto files are off-limits.** Contributors never touch `.proto` files — only Adafruit staff
2535
> modify those. The existing `SensorType` enum already covers all common readings.
2636
@@ -57,28 +67,31 @@ Before writing any code, gather this information:
5767
These are the valid values for `subcomponents` in `definition.json` and map 1:1 to `getEvent*()`
5868
methods in the base driver class:
5969

60-
| Subcomponent | getEvent method | SI unit |
61-
|---|---|---|
62-
| `ambient-temp` | `getEventAmbientTemp` | °C |
63-
| `ambient-temp-fahrenheit` | `getEventAmbientTempF` | °F |
64-
| `humidity` | `getEventRelativeHumidity` | %RH |
65-
| `pressure` | `getEventPressure` | hPa |
66-
| `altitude` | `getEventAltitude` | m |
67-
| `co2` | `getEventCO2` | ppm |
68-
| `eco2` | `getEventECO2` | ppm |
69-
| `tvoc` | `getEventTVOC` | ppb |
70-
| `gas-resistance` | `getEventGasResistance` | Ω |
71-
| `light` | `getEventLight` | lux |
72-
| `proximity` | `getEventProximity` | unitless |
73-
| `voltage` | `getEventVoltage` | V |
74-
| `current` | `getEventCurrent` | A |
75-
| `raw` | `getEventRaw` | unitless |
76-
| `pm10-std` | `getEventPM10_STD` | µg/m³ |
77-
| `pm25-std` | `getEventPM25_STD` | µg/m³ |
78-
| `pm100-std` | `getEventPM100_STD` | µg/m³ |
79-
| `unitless-percent` | `getEventUnitlessPercent` | % |
80-
| `object-temp` | `getEventObjectTemp` | °C |
81-
| `object-temp-fahrenheit` | `getEventObjectTempF` | °F |
70+
| Subcomponent | getEvent method | `sensors_event_t` field | SI unit |
71+
|---|---|---|---|
72+
| `ambient-temp` | `getEventAmbientTemp` | `.temperature` | °C |
73+
| `ambient-temp-fahrenheit` | `getEventAmbientTempF` | `.temperature` | °F |
74+
| `humidity` | `getEventRelativeHumidity` | `.relative_humidity` | %RH |
75+
| `pressure` | `getEventPressure` | `.pressure` | hPa |
76+
| `altitude` | `getEventAltitude` | `.altitude` | m |
77+
| `co2` | `getEventCO2` | `.CO2` | ppm |
78+
| `eco2` | `getEventECO2` | `.eCO2` | ppm |
79+
| `tvoc` | `getEventTVOC` | `.tvoc` | ppb |
80+
| `gas-resistance` | `getEventGasResistance` | `.gas_resistance` | Ω |
81+
| `light` | `getEventLight` | `.light` | lux |
82+
| `proximity` | `getEventProximity` | `.data[0]` | unitless |
83+
| `voltage` | `getEventVoltage` | `.voltage` | V |
84+
| `current` | `getEventCurrent` | `.current` | A |
85+
| `raw` | `getEventRaw` | `.data[0]` | unitless |
86+
| `pm10-std` | `getEventPM10_STD` | `.data[0]` | µg/m³ |
87+
| `pm25-std` | `getEventPM25_STD` | `.data[0]` | µg/m³ |
88+
| `pm100-std` | `getEventPM100_STD` | `.data[0]` | µg/m³ |
89+
| `unitless-percent` | `getEventUnitlessPercent` | `.data[0]` | % |
90+
| `object-temp` | `getEventObjectTemp` | `.temperature` | °C |
91+
| `object-temp-fahrenheit` | `getEventObjectTempF` | `.temperature` | °F |
92+
93+
When using raw reads (not Unified Sensor `getEvent()`), assign to the correct field above, e.g.
94+
`tempEvent->temperature = _sensor->readTempC();`
8295

8396
Temperature sensors almost always include both `ambient-temp` and `ambient-temp-fahrenheit`.
8497

@@ -162,7 +175,12 @@ bool begin() {
162175

163176
### Then: Write the driver using this template
164177

165-
This is a header-only class. Use the closest existing driver as a template. The pattern is:
178+
This is a header-only class. Use the closest existing driver as a template, but **do not blindly
179+
copy** — older drivers may lack the caching and explicit-defaults patterns described above.
180+
Always apply the patterns from this skill even if the closest driver doesn't use them.
181+
182+
> **Note:** Some existing drivers (e.g. TMP117, MCP9808) use simpler patterns that predate
183+
> current best practices. Follow this template, not those older drivers.
166184
167185
```cpp
168186
/*!
@@ -192,60 +210,60 @@ This is a header-only class. Use the closest existing driver as a template. The
192210
/**************************************************************************/
193211
class WipperSnapper_I2C_Driver_<SENSOR> : public WipperSnapper_I2C_Driver {
194212
public:
195-
/*******************************************************************************/
196-
/*!
197-
@brief Constructor for a <SENSOR> sensor.
198-
@param i2c
199-
The I2C interface.
200-
@param sensorAddress
201-
7-bit device address.
202-
*/
203-
/*******************************************************************************/
204213
WipperSnapper_I2C_Driver_<SENSOR>(TwoWire *i2c, uint16_t sensorAddress)
205214
: WipperSnapper_I2C_Driver(i2c, sensorAddress) {
206215
_i2c = i2c;
207216
_sensorAddress = sensorAddress;
208217
}
209218

210-
/*******************************************************************************/
211-
/*!
212-
@brief Destructor for a <SENSOR> sensor.
213-
*/
214-
/*******************************************************************************/
215-
~WipperSnapper_I2C_Driver_<SENSOR>() {
216-
delete _<sensor_ptr>;
217-
}
219+
~WipperSnapper_I2C_Driver_<SENSOR>() { delete _<sensor_ptr>; }
218220

219-
/*******************************************************************************/
220-
/*!
221-
@brief Initializes the <SENSOR> sensor and begins I2C.
222-
@returns True if initialized successfully, False otherwise.
223-
*/
224-
/*******************************************************************************/
225221
bool begin() {
226222
_<sensor_ptr> = new <Adafruit_Class>();
227-
return _<sensor_ptr>->begin((uint8_t)_sensorAddress, _i2c);
223+
if (!_<sensor_ptr>->begin((uint8_t)_sensorAddress, _i2c))
224+
return false;
225+
// Pin library defaults explicitly (found by reading library _init/begin):
226+
// _<sensor_ptr>->setMeasurementMode(...);
227+
// _<sensor_ptr>->setAveragedSampleCount(...);
228+
return true;
228229
}
229230

230231
// --- One getEvent*() per sensor reading type ---
232+
// All go through _readSensor() for caching
231233

232-
/*******************************************************************************/
233-
/*!
234-
@brief Gets the <SENSOR>'s current temperature.
235-
@param tempEvent
236-
Pointer to an Adafruit_Sensor event.
237-
@returns True if the temperature was obtained successfully, False
238-
otherwise.
239-
*/
240-
/*******************************************************************************/
241234
bool getEventAmbientTemp(sensors_event_t *tempEvent) {
242-
// Use the library's getEvent() if it fills sensors_event_t directly,
243-
// otherwise set tempEvent->temperature = _<sensor_ptr>->readTempC();
244-
return _<sensor_ptr>->getEvent(tempEvent);
235+
if (!_readSensor())
236+
return false;
237+
*tempEvent = _cachedTemp;
238+
return true;
245239
}
246240

247241
protected:
248242
<Adafruit_Class> *_<sensor_ptr>; ///< Pointer to <SENSOR> sensor object
243+
244+
// Cached readings and time guard
245+
sensors_event_t _cachedTemp = {0};
246+
unsigned long _lastRead = 0;
247+
248+
/*******************************************************************************/
249+
/*!
250+
@brief Reads sensor data, with 1-second cache to avoid redundant
251+
I2C reads when multiple getEvent*() calls occur per cycle
252+
(e.g. °C then °F, or temp then humidity). The calling order
253+
of getEvent methods is not guaranteed, so every method must
254+
go through this function.
255+
@returns True if cached data is available or a fresh read succeeded.
256+
*/
257+
/*******************************************************************************/
258+
bool _readSensor() {
259+
if (_lastRead != 0 && millis() - _lastRead < 1000)
260+
return true; // use cached values
261+
// Do actual I2C read — adapt to library API:
262+
if (!_<sensor_ptr>->getEvent(&_cachedTemp))
263+
return false;
264+
_lastRead = millis();
265+
return true;
266+
}
249267
};
250268

251269
#endif // WipperSnapper_I2C_Driver_<SENSOR>_H
@@ -402,16 +420,13 @@ This step uses a separate repository: `https://github.com/adafruit/Wippersnapper
402420

403421
### 5a. Fork and clone
404422

405-
If the user needs to fork:
406-
```bash
407-
gh repo fork adafruit/Wippersnapper_Components --clone=false --remote-name origin
408-
gh repo clone <username>/Wippersnapper_Components -- Wippersnapper_Components
409-
```
410-
411-
If they want to use a specific GitHub account (e.g. `tyeth-ai-assisted`), switch with:
423+
Clone inside the firmware repo (it's in `.gitignore`):
412424
```bash
425+
# If using a specific GitHub account:
413426
gh auth switch --user <username>
414-
```
427+
428+
# Fork and clone into the firmware repo root:
429+
gh repo fork adafruit/Wippersnapper_Components --clone=true --remote-name upstream -- Wippersnapper_Components
415430

416431
### 5b. Create component folder
417432

0 commit comments

Comments
 (0)