Skip to content

fix: extract score/direction from MQTT, strip units, fix type safety issues#371

Merged
KpaBap merged 1 commit intomasterfrom
fix/issue-353-mqtt-fields
Apr 1, 2026
Merged

fix: extract score/direction from MQTT, strip units, fix type safety issues#371
KpaBap merged 1 commit intomasterfrom
fix/issue-353-mqtt-fields

Conversation

@Kpa-clawbot
Copy link
Copy Markdown
Owner

Summary

Fixes #353 — addresses all 5 findings from the CoreScope code analysis.

Changes

Finding 1 (Major): score field never extracted from MQTT

  • Added Score *float64 field to PacketData and MQTTPacketMessage structs
  • Extract msg["score"] with msg["Score"] case fallback via toFloat64 in all three MQTT handlers (raw packet, channel message, direct message)
  • Pass through to DB observation insert instead of hardcoded nil

Finding 2 (Major): direction field never extracted from MQTT

  • Added Direction *string field to PacketData and MQTTPacketMessage structs
  • Extract msg["direction"] with msg["Direction"] case fallback as string in all three MQTT handlers
  • Pass through to DB observation insert instead of hardcoded nil

Finding 3 (Minor): toFloat64 doesn't strip units

  • Added stripUnitSuffix() that removes common RF/signal unit suffixes (dBm, dB, mW, km, mi, m) case-insensitively before ParseFloat
  • Values like "-110dBm" or "5.5dB" now parse correctly

Finding 4 (Minor): Bare type assertions in store.go

  • Changed firstSeen and lastSeen from interface{} to typed string variables at store.go:5020
  • Removed unsafe .(string) type assertions in comparisons

Finding 5 (Minor): distHopRecord.SNR typed as interface{}

  • Changed distHopRecord.SNR from interface{} to *float64
  • Updated assignment (removed intermediate snrVal variable, pass tx.SNR directly)
  • Updated output serialization to use floatPtrOrNil(h.SNR) for consistent JSON output

Tests Added

  • TestBuildPacketDataScoreAndDirection — verifies Score/Direction flow through BuildPacketData
  • TestBuildPacketDataNilScoreDirection — verifies nil handling when fields absent
  • TestInsertTransmissionWithScoreAndDirection — end-to-end: inserts with score/direction, verifies DB values
  • TestStripUnitSuffix — covers all supported suffixes, case insensitivity, and passthrough
  • TestToFloat64WithUnits — verifies unit-bearing strings parse correctly

All existing tests pass.

… bare type assertions

Fixes #353:
- Add Score/Direction fields to PacketData and MQTTPacketMessage structs
- Extract score/direction (with case fallback) in all three MQTT handlers
- Pass score/direction through to DB observation insert
- Strip common unit suffixes (dBm, dB, mW, km, mi, m) in toFloat64
- Change firstSeen/lastSeen from interface{} to string in store.go
- Change distHopRecord.SNR from interface{} to *float64
@Kpa-clawbot Kpa-clawbot force-pushed the fix/issue-353-mqtt-fields branch from a532570 to 9eca0b7 Compare April 1, 2026 14:24
@Kpa-clawbot
Copy link
Copy Markdown
Owner Author

Final independent review

I rebased this PR onto master, resolved the conflict in db_test.go (kept both the migration test from master and the new score/direction tests from this branch), verified all tests pass, and reviewed the entire diff from scratch.

Verdict: Approve

The changes are correct, well-tested, and solve the stated problem cleanly.

What's good

  • store.go type safety fixes are excellent — changing distHopRecord.SNR from interface{} to *float64 and firstSeen/lastSeen from interface{} to string eliminates bare type assertions that could panic. Using floatPtrOrNil() for JSON serialization is the right pattern.
  • stripUnitSuffix is well-designed — suffix ordering is correct (longer "dBm" before shorter "dB"/"m"), case-insensitive, and the test coverage is thorough.
  • Test coverage is solidTestBuildPacketDataScoreAndDirection, nil case, round-trip through InsertTransmission, stripUnitSuffix table-driven tests, and toFloat64 with unit strings.
  • Case-insensitive field names ("score" / "Score") match the existing SNR/RSSI pattern.

Observations (non-blocking)

  1. DRY: score/direction extraction is duplicated 3 times in main.go (raw packet handler, group message handler, text message handler). However, SNR/RSSI extraction was already duplicated 3 times before this PR — so this follows the existing pattern. A future cleanup PR should extract a helper like extractSignalFields(msg) (snr, rssi, score *float64, direction *string) to DRY up all 3 blocks, but that's out of scope here.
  2. stripUnitSuffix suffix "m" — matches any string ending in "m". In the current call site (inside toFloat64, applied to values expected to be numeric), this is safe. Just noting it for awareness.

No correctness issues, no edge case gaps, no test coverage holes. Ship it.

@KpaBap KpaBap merged commit be313f6 into master Apr 1, 2026
3 checks passed
@KpaBap KpaBap deleted the fix/issue-353-mqtt-fields branch April 1, 2026 14:26
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.

Audit: additional type-handling vulnerabilities similar to #350

2 participants