Skip to content

Fix/decoder substring matching - #1057

Open
jona159 wants to merge 27 commits into
devfrom
fix/decoder-substring-matching
Open

Fix/decoder substring matching#1057
jona159 wants to merge 27 commits into
devfrom
fix/decoder-substring-matching

Conversation

@jona159

@jona159 jona159 commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Type of Change

  • Dependency upgrade
  • Bug fix (non-breaking change)
  • Breaking change
    • e.g. a fixed bug or new feature that may break something else
  • New feature
  • Code quality improvements
    • e.g. refactoring, documentation, tests, tooling, ...

Implementation

Checklist

  • I gave this pull request a meaningful title
  • My pull request is targeting the dev branch
  • I have added documentation to my code
  • I have deleted code that I have commented out

Additional Information

  • This PR closes #

@coderabbitai

coderabbitai Bot commented Aug 12, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 929fc407-93c3-477b-b08a-57a133ae4d8c

📥 Commits

Reviewing files that changed from the base of the PR and between 1d8eefc and 8a4ac2b.

📒 Files selected for processing (4)
  • tests/routes/api.boxes.data.spec.ts
  • tests/routes/api.device.sensors.spec.ts
  • tests/routes/api.users.me.boxes.$deviceId.spec.ts
  • tests/routes/api.users.me.boxes.spec.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Summary by CodeRabbit

  • New Features

    • Added support for hackAIR Home V2 devices.
    • Expanded sensor coverage with SEN55 and SPS30 measurements.
    • Added sensor templates, metadata, and improved Sensor.Community labeling.
    • Improved decoding for aliases, mappings, multipliers, and shared readings.
  • Bug Fixes

    • Added validation for incompatible sensor models, templates, and ambiguous mappings.
    • Preserved sensor metadata and tags when creating devices.
    • Prevented invalid or duplicate sensor readings from being assigned.
  • Localization

    • Added English and German messages for sensor-mapping conflicts.

Walkthrough

The change adds typed sensor metadata and dynamic model definitions, validates sensor-template combinations, stores catalog identifiers on device sensors, and routes Luftdaten and hackAIR readings through definition mappings. The device setup UI now detects and displays conflicting sensor selections.

Changes

Sensor definition routing

Layer / File(s) Summary
Sensor catalog contract
app/lib/sensor-definitions.ts, app/lib/model-definitions.ts, app/lib/device-enums.ts, app/db/schema/enum.ts, app/lib/api-schemas/devices.ts, app/services/device-service.server.ts
Sensor templates now include typed metadata and decoder mappings. Model definitions are derived dynamically, and sensor-template validation reports unsupported or ambiguous combinations. Shared model enums are used by API and service schemas.
Device sensor metadata
app/db/models/device.server.ts, app/routes/api.boxes.ts, app/routes/device.dashboard.$deviceId.tsx, tests/db/models/device.server.spec.ts, tests/routes/api.boxes.data.spec.ts, tests/routes/api.users.me.boxes.$deviceId.spec.ts, tests/routes/api.users.me.boxes.spec.ts
Device creation resolves validated templates and stores normalized sensor metadata. Tag merging and notification-model handling are updated. Fixtures include sensor templates, and the integration test verifies catalog metadata persistence.
Decoder mapping resolution
app/services/decoding-service.server.ts, tests/services/decoding-service.server.spec.ts, tests/services/hackair-decoding-service.server.spec.ts
Luftdaten and hackAIR decoders prefer exact and definition-based mappings, handle ambiguity without throwing, apply multipliers, and retain the first reading for duplicate destinations.
Sensor selection UI
app/components/device/new/device-info.tsx, app/components/device/new/sensors-info.tsx, public/locales/en/newdevice.json, public/locales/de/newdevice.json
Sensor.Community display handling is separated from the stored model value. Sensor selection uses IDs and shows localized conflict alerts. Test credentials now use generated values.

Estimated code review effort: 4 (Complex) | ~45 minutes

Merge Risk: 🟠 High · up to 8a4ac

The PR changes sensor decoding and device creation behavior, but the current version can misidentify sensors, store CO2 readings under the wrong phenomenon, reject valid modeled devices, persist conflicting routing information, and break existing or newly modeled device records. These high-impact correctness and compatibility risks should be fixed before merging.

Sequence Diagram(s)

sequenceDiagram
  participant DeviceSetup
  participant CreateDeviceSchema
  participant modelDefinitions
  participant DeviceService
  participant DeviceSensors
  DeviceSetup->>CreateDeviceSchema: submit model and sensorTemplates
  CreateDeviceSchema->>modelDefinitions: validate sensor-template combination
  modelDefinitions-->>CreateDeviceSchema: return validation result
  CreateDeviceSchema->>DeviceService: pass validated device data
  DeviceService->>DeviceSensors: resolve and store sensor metadata
  DeviceSensors-->>DeviceService: return created sensors
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Description check ❓ Inconclusive The description contains only an uncompleted template and provides no meaningful details about the changeset. Describe the implementation, affected behavior, and purpose of the decoder, sensor mapping, and validation changes.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the decoder substring-matching changes but does not cover the broader sensor validation and mapping updates.
Docstring Coverage ✅ Passed Docstring check was indeterminate for this PR — some files could not be analyzed in time. Not blocking.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/decoder-substring-matching

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Aug 12, 2026

Copy link
Copy Markdown

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 68.33% 2393 / 3502
🔵 Statements 66.94% 2481 / 3706
🔵 Functions 65.39% 463 / 708
🔵 Branches 54.04% 1196 / 2213
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
app/db/models/device.server.ts 61.79% 64.97% 59.18% 63.07% 94-99, 107, 214, 226-227, 292-349, 389, 423, 452, 481, 511, 515, 523-525, 531-533, 551-553, 557-559, 601-603, 618, 629-801, 852-856, 884-890, 895-911, 995-997, 1006, 1017, 1028-1035, 1039-1050, 1090, 1122, 1217-1237, 1256
app/db/schema/enum.ts 100% 100% 100% 100%
app/lib/device-enums.ts 60% 0% 0% 60% 35-43
app/lib/model-definitions.ts 89.09% 81.57% 100% 89.36% 170, 188-191, 200, 203, 213, 219
app/lib/sensor-definitions.ts 100% 100% 100% 100%
app/lib/api-schemas/devices.ts 66.66% 65.38% 70% 65.11% 48-54, 63-67, 98, 109, 118-121, 132-149, 157, 177-182
app/routes/api.boxes.ts 95.45% 95% 100% 95.45% 205, 258
app/services/decoding-service.server.ts 80.98% 67.17% 100% 84.32% 57, 70-72, 81-82, 84-85, 88, 92, 93, 117-120, 136, 145, 184, 189, 212-213, 230-231, 240, 249-250, 260-278, 331-335, 351, 352, 354, 366, 375, 397, 398, 400, 409, 435, 440-442, 471, 476-478, 482, 537-539, 588-590
app/services/device-service.server.ts 26.98% 19.35% 16% 27.58% 49-63, 73-78, 84-192, 210, 227, 249-252
Generated in workflow #2904 for commit 4afab93 by the Vitest Coverage Report Action

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (2)
app/db/models/device.server.ts (1)

1095-1112: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win

Validate a caller-supplied sensorDefinitionId on custom devices.

The last branch stores sensorData.data unchanged. A client that creates a custom device can therefore set data.sensorDefinitionId to any catalog key. findLuftdatenSensorMapping accepts that key and applies its mapping, including the 0.01 pressure multiplier, so stored measurement values change silently.

Either strip unknown sensorDefinitionId values in this branch, or validate the key against sensorDefinitions before persisting it.

♻️ Proposed validation
+					const requestedDefinitionId = existingSensorData.sensorDefinitionId
+					const isKnownDefinitionId =
+						typeof requestedDefinitionId === 'string' &&
+						requestedDefinitionId in sensorDefinitions
 					const sensorMetadata = storedDeviceSchemaVersion
 						? {
 								...existingSensorData,
 								deviceSchemaSensorId: sensorData.id,
 							}
 						: usesSensorDefinitions
 							? {
 									...existingSensorData,
 									sensorDefinitionId: sensorData.id,
 								}
-							: sensorData.data
+							: requestedDefinitionId !== undefined && !isKnownDefinitionId
+								? { ...existingSensorData, sensorDefinitionId: undefined }
+								: sensorData.data
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/db/models/device.server.ts` around lines 1095 - 1112, Validate the
caller-supplied sensorDefinitionId in the final branch of the sensorMetadata
construction before persisting sensorData.data. Accept it only when it matches a
key in sensorDefinitions; otherwise remove or ignore that field while preserving
the rest of the custom sensor data. Keep the storedDeviceSchemaVersion and
usesSensorDefinitions branches unchanged.
tests/services/decoding-service.server.spec.ts (1)

105-159: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add coverage for mixed sensor sets and duplicate catalog value types.

Two paths stay untested. First, a device that has both catalog sensors and legacy title-only sensors in one request. Second, a device that has two sensors whose definitions claim the same Luftdaten value type, for example pms5003_pm01 and pms7003_pm01. The second case is the failure described in app/lib/sensor-definitions.ts. A test would pin the intended behaviour.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/services/decoding-service.server.spec.ts` around lines 105 - 159,
Extend the decodeMeasurements test suite with coverage for a request containing
both catalog-defined sensors and legacy title-only sensors, asserting each maps
correctly. Add a separate test with sensor definitions such as pms5003_pm01 and
pms7003_pm01 that resolve to the same Luftdaten value type, and assert the
intended duplicate-mapping behavior described by sensor-definitions.ts.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@app/lib/sensor-definitions.ts`:
- Around line 417-437: Update the Luftdaten entries in sensorDefinitions so
findLuftdatenSensorMapping can resolve each affected valueType—PMS_P0, PMS_P1,
PMS_P2, BME280_pressure, BMP180_pressure, temperature, and humidity—without
ambiguity when all model sensors are added. Ensure each value type has exactly
one mapping, or add deterministic discriminator metadata such as sensor type or
unit, while preserving the intended sensor phenomena.

In `@app/services/decoding-service.server.ts`:
- Around line 90-122: Change the ambiguity handling across
app/services/decoding-service.server.ts:90-122, :188-211, and :352-383 so one
unresolved value is skipped without aborting the full upload. In the Luftdaten
resolver at :90-122 and findHackairSensorId at :188-211, return undefined or
record the conflict instead of throwing; in the duplicate
destination-measurement handling at :352-383, drop the later duplicate after
this non-throwing policy is applied, preserving other measurements in the
request.

---

Nitpick comments:
In `@app/db/models/device.server.ts`:
- Around line 1095-1112: Validate the caller-supplied sensorDefinitionId in the
final branch of the sensorMetadata construction before persisting
sensorData.data. Accept it only when it matches a key in sensorDefinitions;
otherwise remove or ignore that field while preserving the rest of the custom
sensor data. Keep the storedDeviceSchemaVersion and usesSensorDefinitions
branches unchanged.

In `@tests/services/decoding-service.server.spec.ts`:
- Around line 105-159: Extend the decodeMeasurements test suite with coverage
for a request containing both catalog-defined sensors and legacy title-only
sensors, asserting each maps correctly. Add a separate test with sensor
definitions such as pms5003_pm01 and pms7003_pm01 that resolve to the same
Luftdaten value type, and assert the intended duplicate-mapping behavior
described by sensor-definitions.ts.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: b15712b9-478f-40a6-887c-b42e50b19399

📥 Commits

Reviewing files that changed from the base of the PR and between 5635799 and 563147c.

📒 Files selected for processing (7)
  • app/db/models/device.server.ts
  • app/lib/model-definitions.ts
  • app/lib/sensor-definitions.ts
  • app/services/decoding-service.server.ts
  • tests/db/models/device.server.spec.ts
  • tests/services/decoding-service.server.spec.ts
  • tests/services/hackair-decoding-service.server.spec.ts

Comment thread app/lib/sensor-definitions.ts
Comment thread app/services/decoding-service.server.ts
@jona159
jona159 marked this pull request as ready for review August 21, 2026 06:44

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
app/services/decoding-service.server.ts (1)

180-216: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Route hackAIR readings by sensor definition ID.

modelDefinitions.hackair_home_v2 selects sds011_pm10 and sds011_pm25, and device creation stores these IDs in sensor.data.sensorDefinitionId. Resolve these IDs before the title-based fallback.

Source: Linters/SAST tools

🧹 Nitpick comments (4)
app/lib/model-definitions.ts (1)

73-73: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add satisfies readonly SensorKey[] to hackair_home_v2.

Every other literal model list in this object uses as const satisfies readonly SensorKey[]. Without it, a typo in a sensor key in hackair_home_v2 is not caught at compile time, and sensorDefinitions[key] resolves to undefined at runtime.

♻️ Proposed change
-	hackair_home_v2: ['sds011_pm10', 'sds011_pm25'] as const,
+	hackair_home_v2: [
+		'sds011_pm10',
+		'sds011_pm25',
+	] as const satisfies readonly SensorKey[],
app/services/device-service.server.ts (1)

53-65: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Align this refinement with CreateDeviceSchema.

If a caller sends model: 'luftdaten.info' together with sensors, the .refine at Line 49 reports "Model and sensors cannot be specified at the same time.", and this superRefine also reports "At least one sensor template is required for model luftdaten.info". The second message is misleading because sensor templates are not relevant when sensors are given.

CreateDeviceSchema in app/lib/api-schemas/devices.ts skips template validation when sensors are present. Apply the same skip here.

♻️ Proposed change
 	.superRefine((data, ctx) => {
+		if (data.sensors?.length) return
+
 		const message = getSensorTemplateValidationError(
 			data.model,
 			data.sensorTemplates,
 		)
app/db/models/device.server.ts (2)

1013-1018: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Remove the unreachable model check.

getSensorsForModel always returns an array, so !Array.isArray(modelSensors) is never true. Unknown models are already rejected by getSensorTemplateValidationError at Line 1002, which returns Unknown model: .... This block is dead code.

♻️ Proposed change
-				if (
-					!Array.isArray(modelSensors) &&
-					deviceData.model?.toLowerCase() !== 'custom'
-				) {
-					throw new Error(`Unknown model: ${deviceData.model}`)
-				}
-
 				sensorsToAdd = modelSensors

1098-1122: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Reuse existingSensorData in the last branch.

Lines 1098-1103 already compute whether sensorData.data is a plain object and store the result in existingSensorData. Lines 1114-1116 repeat the same three checks. A single helper removes the duplication and keeps the three branches readable.

♻️ Proposed change
-					const existingSensorData =
-						sensorData.data &&
-						typeof sensorData.data === 'object' &&
-						!Array.isArray(sensorData.data)
-							? sensorData.data
-							: {}
+					const isPlainSensorData =
+						Boolean(sensorData.data) &&
+						typeof sensorData.data === 'object' &&
+						!Array.isArray(sensorData.data)
+					const existingSensorData = isPlainSensorData ? sensorData.data : {}
 					const sensorMetadata = storedDeviceSchemaVersion
 						? {
 								...existingSensorData,
 								deviceSchemaSensorId: sensorData.id,
 							}
 						: usesSensorDefinitions
 							? {
 									...existingSensorData,
 									sensorDefinitionId: sensorData.id,
 								}
-							: sensorData.data &&
-								  typeof sensorData.data === 'object' &&
-								  !Array.isArray(sensorData.data)
+							: isPlainSensorData
 								? Object.fromEntries(
 										Object.entries(sensorData.data).filter(
 											([key]) => key !== 'sensorDefinitionId',
 										),
 									)
 								: sensorData.data

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 2499c1e4-d320-4fc8-9b80-7cc2ef3ae291

📥 Commits

Reviewing files that changed from the base of the PR and between 563147c and 2a112d4.

📒 Files selected for processing (14)
  • app/components/device/new/device-info.tsx
  • app/components/device/new/sensors-info.tsx
  • app/db/models/device.server.ts
  • app/db/schema/enum.ts
  • app/lib/api-schemas/devices.ts
  • app/lib/device-enums.ts
  • app/lib/model-definitions.ts
  • app/lib/sensor-definitions.ts
  • app/routes/api.boxes.ts
  • app/routes/device.dashboard.$deviceId.tsx
  • app/services/decoding-service.server.ts
  • app/services/device-service.server.ts
  • public/locales/de/newdevice.json
  • public/locales/en/newdevice.json

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.

Comment thread app/components/device/new/sensors-info.tsx
Comment thread app/lib/device-enums.ts
Comment on lines +12 to +24
export const DEVICE_MODEL_VALUES = [
'homeV2Lora',
'homeV2Ethernet',
'homeV2Wifi',
'homeEthernet',
'homeWifi',
'homeEthernetFeinstaub',
'homeWifiFeinstaub',
'luftdaten.info',
'hackair_home_v2',
'senseBox:Edu',
'custom',
] as const

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- migrations that define or alter the model enum ---'
rg -n -i "TYPE \"?model\"?|ADD VALUE|luftdaten_sds011|hackair" --glob '*.sql' .

echo '--- model keys in model-definitions ---'
rg -n "^\t[A-Za-z'\"].*:" app/lib/model-definitions.ts | sed -n '1,80p'

echo '--- other references to removed model values ---'
rg -n "luftdaten_sds011|homeV2EthernetFeinstaub|homeV2WifiFeinstaub|senseBoxHomeV2" app

Repository: openSenseMap/frontend

Length of output: 14314


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- device enum and schema declarations ---'
cat -n app/lib/device-enums.ts
cat -n app/db/schema/enum.ts

echo '--- model definitions ---'
cat -n app/lib/model-definitions.ts | sed -n '1,115p'

echo '--- device model handling ---'
rg -n -C 4 "luftdaten_sds011|homeV2EthernetFeinstaub|homeV2WifiFeinstaub|senseBoxHomeV2|DEVICE_MODEL_VALUES|modelEnum|model:" app/db/models/device.server.ts app

echo '--- migration files after 0027 ---'
git ls-files 'app/db/drizzle/*.sql' | sort -V | tail -20

Repository: openSenseMap/frontend

Length of output: 50377


🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
import re
from pathlib import Path

enum_text = Path("app/lib/device-enums.ts").read_text()
sql_text = "\n".join(p.read_text() for p in Path("app/db/drizzle").glob("*.sql"))
defs_text = Path("app/lib/model-definitions.ts").read_text()

list_values = re.findall(r"^\s*(['\"])(.*?)\1,\s*$", enum_text, re.M)
list_values = [v for _, v in list_values]
sql_values = sorted(set(re.findall(r"'([^']+)'", sql_text)))
definition_keys = re.findall(r"^\s*(['\"]?[\w:.]+['\"]?)\s*:", defs_text, re.M)
definition_keys = [k.strip("'\"") for k in definition_keys]

print("DEVICE_MODEL_VALUES:", list_values)
print("SQL enum-related values:", sql_values)
print("SQL values absent from DEVICE_MODEL_VALUES:",
      sorted(set(sql_values) - set(list_values)))
print("DEVICE_MODEL_VALUES absent from SQL values:",
      sorted(set(list_values) - set(sql_values)))
print("model definition keys:", definition_keys)
print("definition keys absent from DEVICE_MODEL_VALUES:",
      sorted(set(definition_keys) - set(list_values)))
PY

echo '--- current schema usage of DEVICE_MODEL_VALUES ---'
rg -n -C 5 "DEVICE_MODEL_VALUES|model.*enum|enum.*model|ModelDefinitionKey" app/db app/lib app | sed -n '1,240p'

Repository: openSenseMap/frontend

Length of output: 13508


🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
import re
from pathlib import Path

device_values = re.findall(
    r"^\s*(['\"])(.*?)\1,\s*$",
    Path("app/lib/device-enums.ts").read_text(),
    re.M,
)
device_values = [value for _, value in device_values[4:]]  # skip exposure values

# Inspect only SQL statements that define or replace the public model enum.
for path in sorted(Path("app/db/drizzle").glob("*.sql")):
    text = path.read_text()
    matches = re.findall(
        r"(?:CREATE TYPE\s+(?:\"public\"\.)?\"model\"\s+AS\s+ENUM|"
        r"ALTER TYPE\s+(?:\"public\"\.)?\"model\"\s+ADD VALUE)[^;]*",
        text,
        re.I,
    )
    if matches:
        print(path)
        for match in matches:
            print(" ", match.strip())

print("\nMissing historical model enum values:")
historical = {
    "luftdaten_sds011",
    "luftdaten_sds011_dht11",
    "luftdaten_sds011_dht22",
    "luftdaten_sds011_bmp180",
    "luftdaten_sds011_bme280",
}
print(sorted(historical - set(device_values)))

print("\nMissing persisted model-definition keys:")
definition_block = Path("app/lib/model-definitions.ts").read_text().split(
    "export const modelDefinitions = {", 1
)[1].split("} as const", 1)[0]
definition_keys = re.findall(r"^\s*(?:'([^']+)'|([A-Za-z0-9_:.]+))\s*:", definition_block, re.M)
definition_keys = {a or b for a, b in definition_keys}
print(sorted({"homeV2EthernetFeinstaub", "homeV2WifiFeinstaub"} - set(device_values)))
print("senseBoxHomeV2 is an internal modelDefinitions key:", "senseBoxHomeV2" in definition_keys)
PY

echo '--- validation and creation path ---'
cat -n app/lib/api-schemas/devices.ts | sed -n '20,40p'
cat -n app/routes/api.boxes.ts | sed -n '215,245p'
cat -n app/db/models/device.server.ts | sed -n '995,1075p'

Repository: openSenseMap/frontend

Length of output: 6796


🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
import re
from pathlib import Path

text = Path("app/lib/device-enums.ts").read_text()
block = text.split("export const DEVICE_MODEL_VALUES = [", 1)[1].split("] as const", 1)[0]
values = re.findall(r"['\"]([^'\"]+)['\"]", block)

historical = {
    "luftdaten_sds011",
    "luftdaten_sds011_dht11",
    "luftdaten_sds011_dht22",
    "luftdaten_sds011_bmp180",
    "luftdaten_sds011_bme280",
}
persisted_definitions = {
    "homeV2Lora",
    "homeV2Ethernet",
    "homeV2EthernetFeinstaub",
    "homeV2Wifi",
    "homeV2WifiFeinstaub",
    "senseBox:Edu",
    "luftdaten.info",
    "hackair_home_v2",
    "homeEthernet",
    "homeWifi",
    "homeEthernetFeinstaub",
    "homeWifiFeinstaub",
}

print("DEVICE_MODEL_VALUES:", values)
print("Historical enum values missing:", sorted(historical - set(values)))
print("Persisted model-definition keys missing:", sorted(persisted_definitions - set(values)))

for path in sorted(Path("app/db/drizzle").glob("*.sql")):
    text = path.read_text()
    if re.search(r'(?i)(?:CREATE TYPE|ALTER TYPE).*[".]model', text):
        print(f"{path}:")
        for line in text.splitlines():
            if re.search(r"(?i)(?:CREATE TYPE|ALTER TYPE).*model", line):
                print(" ", line.strip())
PY

Repository: openSenseMap/frontend

Length of output: 2453


Preserve existing model enum values and add the missing persisted model keys.

DeviceModelZodEnum and DeviceModelEnum derive from DEVICE_MODEL_VALUES. Add luftdaten_sds011, luftdaten_sds011_dht11, luftdaten_sds011_dht22, luftdaten_sds011_bmp180, and luftdaten_sds011_bme280. Add homeV2EthernetFeinstaub and homeV2WifiFeinstaub; CreateDeviceSchema currently rejects these values. Do not add senseBoxHomeV2, because it is an internal model-definition alias, not a persisted model enum value.

Comment thread app/lib/model-definitions.ts
Comment on lines +756 to +766
sen55_nox: {
phenomenon: 'nitrogen-oxides-index',
decoderMappings: {
luftdaten: [
{ valueType: 'SHT3X_co2_ppm' },
{ valueType: 'SEN5X_nox' },
{ valueType: 'SEN55_nox' },
{ valueType: 'SEN55_co2_ppm' },
],
},
},

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

sensor.community Luftdaten API value types SEN5X SEN55 nox voc names

💡 Result:

The Sensor.Community (formerly Luftdaten) API does not currently support nox (nitrogen oxides) or voc (volatile organic compounds) as valid value types [1][2]. Although the Sensirion SEN5x series (including the SEN55) natively measures VOC and NOx [3], attempting to push these values to the official Sensor.Community API will result in a 400 error, with the server rejecting them as invalid choices [1][2][4]. Key points regarding SEN5x integration: 1. Value Type Rejection: Users who attempt to send nox or voc via the API receive a response indicating that these are not valid choices [1][2]. The API is primarily designed for particulate matter (e.g., P0, P1, P2) and standard environmental metrics (e.g., temperature, humidity) [2][5][4]. 2. Firmware Constraints: While some custom firmware may include these sensors, the data fields for NOx and VOC are generally stripped or ignored before transmission to the official platform [6][2]. 3. Data Limitations: Community discussion highlights that even if the API were updated, NOx and VOC readings from these sensors are often provided as indexes rather than absolute concentrations, which complicates their presentation on public platforms [6][2]. 4. Alternative Solutions: Users who require NOx and VOC data from SEN5x sensors typically use their own infrastructure (e.g., personal MQTT servers or alternative data logging platforms) rather than the Sensor.Community API [6]. If you are developing custom firmware for this device, ensure your sensordatavalues payload only includes supported types (such as PM values, temperature, and humidity) to avoid rejection [2][4]. Adding prefixes like SEN5X_ to the value types is also incorrect; the API expects standard identifiers (e.g., temperature, humidity) [2].

Citations:


🏁 Script executed:

#!/bin/bash
set -eu
printf '%s\n' '--- target definition ---'
sed -n '720,785p' app/lib/sensor-definitions.ts
printf '%s\n' '--- related value types and phenomena ---'
rg -n -C 3 "SHT3X_co2_ppm|SEN5X_nox|SEN55_nox|SEN55_co2_ppm|sen55_nox|nitrogen-oxides-index" .
printf '%s\n' '--- sensor definition structure ---'
ast-grep outline app/lib/sensor-definitions.ts | sed -n '1,220p'

Repository: openSenseMap/frontend

Length of output: 3478


🏁 Script executed:

#!/bin/bash
set -eu
printf '%s\n' '--- mapping types and decoder construction ---'
sed -n '1,70p' app/lib/sensor-definitions.ts
sed -n '350,410p' app/lib/sensor-definitions.ts
sed -n '800,850p' app/lib/sensor-definitions.ts
printf '%s\n' '--- all exact value-type occurrences ---'
rg -n -C 4 "SHT3X_co2_ppm|SEN55_co2_ppm|SEN5X_nox|SEN55_nox" --glob '!node_modules/**' .
printf '%s\n' '--- decoder mapping consumers ---'
rg -n -C 5 "decoderMappings|valueType" app --glob '*.{ts,tsx,js,jsx}' | sed -n '1,280p'

Repository: openSenseMap/frontend

Length of output: 22560


🏁 Script executed:

#!/bin/bash
set -eu
printf '%s\n' '--- decode path ---'
sed -n '90,230p' app/services/decoding-service.server.ts
printf '%s\n' '--- decode tests and fixtures ---'
rg -n -C 5 "findLuftdatenSensorMapping|Luftdaten|SEN55|NOx|nox|co2_ppm|decoderMappings" --glob '*.{test,spec}.{ts,tsx,js,jsx}' --glob '*.{json,yaml,yml}' .
printf '%s\n' '--- repository references to upstream definitions ---'
rg -n -i -C 3 "sensor.community|luftdaten|sensordatavalues|SEN5X|SEN55|SHT3X" README.md docs app package.json .github 2>/dev/null | sed -n '1,260p'

Repository: openSenseMap/frontend

Length of output: 25377


🌐 Web query:

Sensor.Community firmware "SHT3X_co2_ppm" OR "SEN55_co2_ppm" OR "SEN5X_nox"

💡 Result:

In the context of the Sensor.Community project and the standard airRohr firmware, the value types SHT3X_co2_ppm, SEN55_co2_ppm, and SEN5X_nox are not officially supported by the central Sensor.Community API [1][2][3]. Detailed Breakdown: 1. API Compatibility: The Sensor.Community API is designed to accept specific, registered sensor value types (such as PM values, temperature, humidity, and atmospheric pressure) [4][5]. When payloads are sent containing unsupported types like nox, voc, or custom-prefixed values such as SEN5X_nox or SEN5X_voc, the API will return an error (often HTTP 400) indicating that these are not valid choices [1][2][4]. 2. SEN5X Support: While the SEN5x series (e.g., SEN55) can be integrated for particulate matter (PM), temperature, and humidity measurements, the advanced indices (VOC and NOx) provided by these sensors are not natively supported by the main Sensor.Community database or map [6][3]. Users attempting to include these data points in their JSON payloads frequently encounter rejection errors from the backend [1][2]. 3. Workarounds and Limitations: - Standard Integration: Users commonly integrate SEN5x sensors by configuring them as SPS30 (for PM) and SHT3x (for temperature/humidity) in the firmware [6][7][8]. - Data Reporting: Because the official API rejects specialized indices, users who require VOC/NOx data must typically implement their own communication channels (e.g., via MQTT or a private server) rather than relying on the main Sensor.Community platform for these specific measurements [6][8]. - Firmware Configurations: Historical firmware modifications (such as those for the SEN5x or custom DNMS branches) have attempted to send these values; however, these are not compatible with the standard Sensor.Community data ingestion pipeline [2][9][7]. If you are seeing these value types in older or custom firmware, be aware that they are intended for private data analysis or secondary platforms (like Madavi.de) and will not be correctly processed by the main Sensor.Community system [2][9].

Citations:


🏁 Script executed:

#!/bin/bash
set -eu
printf '%s\n' '--- emitted measurement shape ---'
sed -n '340,410p' app/services/decoding-service.server.ts
printf '%s\n' '--- read-only mapping simulation ---'
python3 - <<'PY'
import re
from pathlib import Path

source = Path("app/lib/sensor-definitions.ts").read_text()
block = re.search(
    r"\bsen55_nox:\s*\{\s*phenomenon:\s*'([^']+)'[\s\S]*?"
    r"luftdaten:\s*\[(.*?)\]\s*,\s*\}\s*,\s*\}",
    source,
)
if not block:
    raise SystemExit("sen55_nox mapping block not found")

phenomenon = block.group(1)
value_types = re.findall(r"valueType:\s*'([^']+)'", block.group(2))
print({"phenomenon": phenomenon, "value_types": value_types})

for incoming in value_types:
    matches = [candidate for candidate in value_types
               if candidate.lower() == incoming.lower()]
    print({
        "incoming_value_type": incoming,
        "resolved_sensor_definition": "sen55_nox",
        "stored_phenomenon": phenomenon,
        "parsed_value": 42.0,
        "mapping_match": matches == [incoming],
    })
PY

Repository: openSenseMap/frontend

Length of output: 2430


🏁 Script executed:

#!/bin/bash
set -eu
python3 - <<'PY'
import re
from pathlib import Path

lines = Path("app/lib/sensor-definitions.ts").read_text().splitlines()
start = next(i for i, line in enumerate(lines) if line.strip() == "sen55_nox: {")
end = next(i for i in range(start + 1, len(lines))
           if re.match(r"\s*[A-Za-z0-9_]+:\s*\{", lines[i]))
block = "\n".join(lines[start:end])

phenomenon = re.search(r"phenomenon:\s*'([^']+)'", block).group(1)
value_types = re.findall(r"valueType:\s*'([^']+)'", block)

print("definition:", "sen55_nox")
print("phenomenon:", phenomenon)
print("mappings:", value_types)

for incoming in value_types:
    mapping = next(
        candidate for candidate in value_types
        if candidate.lower() == incoming.lower()
    )
    print({
        "incoming_value_type": incoming,
        "resolved_mapping": mapping,
        "destination_sensor_definition": "sen55_nox",
        "emitted_value": 42.0,
        "declared_phenomenon": phenomenon,
    })
PY

Repository: openSenseMap/frontend

Length of output: 292


🏁 Script executed:

#!/bin/bash
set -eu
python3 - <<'PY'
import re
from pathlib import Path

lines = Path("app/lib/sensor-definitions.ts").read_text().splitlines()
starts = [i for i, line in enumerate(lines)
          if line.strip() == "sen55_nox: {"]
start = next(i for i in starts if i + 1 >= 700)
end = next(i for i in range(start + 1, len(lines))
           if re.match(r"\s*[A-Za-z0-9_]+:\s*\{", lines[i]))
block = "\n".join(lines[start:end])

phenomenon_match = re.search(r"phenomenon:\s*'([^']+)'", block)
if not phenomenon_match:
    raise SystemExit("metadata phenomenon not found")
phenomenon = phenomenon_match.group(1)
value_types = re.findall(r"valueType:\s*'([^']+)'", block)

print("definition:", "sen55_nox")
print("phenomenon:", phenomenon)
print("mappings:", value_types)

for incoming in value_types:
    mapping = next(
        candidate for candidate in value_types
        if candidate.lower() == incoming.lower()
    )
    print({
        "incoming_value_type": incoming,
        "resolved_mapping": mapping,
        "destination_sensor_definition": "sen55_nox",
        "emitted_value": 42.0,
        "declared_phenomenon": phenomenon,
    })
PY

Repository: openSenseMap/frontend

Length of output: 228


🏁 Script executed:

#!/bin/bash
set -eu
python3 - <<'PY'
import re
from pathlib import Path

lines = Path("app/lib/sensor-definitions.ts").read_text().splitlines()
# Reviewed range: source lines 756-766, using 1-based line numbers.
block = "\n".join(lines[755:766])

phenomenon = re.search(r"phenomenon:\s*'([^']+)'", block).group(1)
value_types = re.findall(r"valueType:\s*'([^']+)'", block)
expected_nox = {"SEN5X_nox", "SEN55_nox"}
unexpected = set(value_types) - expected_nox

print("phenomenon:", phenomenon)
print("mappings:", value_types)
print("unexpected_mappings:", sorted(unexpected))

for incoming in value_types:
    matched = next(v for v in value_types if v.lower() == incoming.lower())
    print({
        "incoming_value_type": incoming,
        "destination_definition": "sen55_nox",
        "stored_value": 42.0,
        "stored_phenomenon": phenomenon,
        "is_nox_mapping": matched in expected_nox,
    })
PY

Repository: openSenseMap/frontend

Length of output: 1002


Remove the CO₂ mappings from sen55_nox.

The decoder maps SHT3X_co2_ppm and SEN55_co2_ppm to the nitrogen-oxides-index sensor. Keep only SEN5X_nox and SEN55_nox, or document an upstream contract for these mappings.

Comment thread app/services/decoding-service.server.ts

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
app/db/models/device.server.ts (2)

986-987: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Treat an empty sensor array as no explicit sensors.

The API schema uses data.sensors.length, so modeled requests can reach this code with sensors: []. An empty array is truthy in JavaScript. The current conflict check rejects non-custom models before model resolution, and usesSensorDefinitions is also set to false.

Define hasExplicitSensors as Array.isArray(deviceData.sensors) && deviceData.sensors.length > 0, then use it for the conflict check, usesSensorDefinitions, and the model-resolution condition. Add a regression test for a modeled request with sensors: [].

Proposed fix
 			const isCustomDevice =
 				!deviceData.model || deviceData.model?.toLowerCase() === 'custom'
+			const hasExplicitSensors =
+				Array.isArray(deviceData.sensors) && deviceData.sensors.length > 0
 			const usesSensorDefinitions =
-				Boolean(deviceData.model) && !isCustomDevice && !deviceData.sensors
+				Boolean(deviceData.model) && !isCustomDevice && !hasExplicitSensors

 			if (
 				deviceData.model &&
-				deviceData.sensors &&
+				hasExplicitSensors &&
 				deviceData.model.toLowerCase() !== 'custom'
 			) {

-			if (deviceData.model && !deviceData.sensors) {
+			if (deviceData.model && !hasExplicitSensors) {

Also applies to: 1000-1001


1101-1125: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Keep routing identifiers mutually exclusive.

The schema-backed and model-definition branches spread existingSensorData without filtering it. Only the fallback branch removes sensorDefinitionId, and it does not remove deviceSchemaSensorId. A schema or caller-provided object can therefore persist both identifiers. Downstream consumers such as app/components/device/new/sensors-info.tsx can then resolve a sensor through the wrong source.

Remove both routing keys before adding the branch-specific identifier.

Proposed fix
 					const existingSensorData =
 						sensorData.data &&
 						typeof sensorData.data === 'object' &&
 						!Array.isArray(sensorData.data)
-							? sensorData.data
+							? Object.fromEntries(
+									Object.entries(sensorData.data).filter(
+										([key]) =>
+											key !== 'sensorDefinitionId' &&
+											key !== 'deviceSchemaSensorId',
+									),
+								)
 							: {}
🧹 Nitpick comments (1)
app/db/models/device.server.ts (1)

1002-1011: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Make unknown-model validation match getSensorsForModel.

getSensorsForModel returns [] when modelDefinitions[model] is missing. Therefore Array.isArray(modelSensors) is still true for an unknown model, so the check at Lines 1013-1017 cannot reject it. Validate the model key explicitly, or make the resolver return a distinct failure value. Verify that getSensorTemplateValidationError rejects unknown models for every direct createDevice caller before relying on it as the only guard.


ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 1d48dbc8-81a5-4ab2-b6dc-4fbd0993af53

📥 Commits

Reviewing files that changed from the base of the PR and between 2a112d4 and a179724.

📒 Files selected for processing (2)
  • app/components/device/new/device-info.tsx
  • app/db/models/device.server.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant