Add build system, tests, and project documentation#4
Conversation
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Provides per-variant build/upload/flash/monitor targets for all 6 hardware variants, plus LittleFS data upload for variants with audio assets. Uses GNU Make define/eval/call to generate targets from a single template. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Tests compile natively with clang++/g++ — no ESP32 hardware needed. Covers detectors, device tracker state machine, and threat analyzer scoring pipeline (37 cases, 126 assertions) targeting the M5Stick variant's pure-logic headers. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR adds comprehensive build system infrastructure, host-side unit testing, and project documentation to the FlockSquawk embedded surveillance detection system. It builds upon PR #3's pluggable detector system by adding testing capabilities and development tooling.
Changes:
- Introduces a Makefile wrapping arduino-cli for building, uploading, and testing across 6 hardware variants
- Adds host-side unit test infrastructure using doctest with 290+ test cases covering detectors, device tracking, and threat analysis
- Provides CLAUDE.md with architecture reference and development guidance for AI-assisted development
Reviewed changes
Copilot reviewed 16 out of 17 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| Makefile | Arduino-cli wrapper with variant-specific targets, test runner, and dependency installer |
| CLAUDE.md | Project architecture, build instructions, and thread safety patterns documentation |
| .gitignore | Excludes build artifacts and downloaded doctest header |
| test/test_main.cpp | Doctest test runner entry point |
| test/mocks/Arduino.h | Minimal Arduino API mock for host compilation (millis, constrain, string functions) |
| test/eventbus_impl.cpp | EventBus static member storage and mock implementations for tests |
| test/test_detectors.cpp | Unit tests for detector functions (SSID/UUID matching, OUI lookup, RSSI modifiers) |
| test/test_device_tracker.cpp | Unit tests for device tracking state machine and LRU eviction |
| test/test_threat_analyzer.cpp | Integration tests for scoring pipeline, subsumption, and alert logic |
| m5stack/flocksquawk_m5stick/src/ThreatAnalyzer.h | Moved implementation from .cpp to inline header for testability; added DeviceTracker class |
| m5stack/flocksquawk_m5stick/src/TelemetryReporter.h | Moved implementation to inline header; updated JSON output for new detector fields |
| m5stack/flocksquawk_m5stick/src/EventBus.h | Changed ThreatEvent radioType/category from pointers to fixed arrays; added detector metadata fields |
| m5stack/flocksquawk_m5stick/src/DeviceSignatures.h | Removed unused NetworkNames, BLEIdentifiers, and RavenServices arrays (moved to Detectors.h) |
| m5stack/flocksquawk_m5stick/src/Detectors.h | New detector function implementations with inline helpers |
| m5stack/flocksquawk_m5stick/src/DetectorTypes.h | Type definitions for detector system (flags, results, device state) |
| m5stack/flocksquawk_m5stick/src/RadioScanner.h | Made currentWifiChannel volatile; doubled channel switch interval to 1000ms |
| m5stack/flocksquawk_m5stick/flocksquawk_m5stick.ino | Added BLE spinlock protection; integrated DeviceTracker tick() for heartbeat beeps; conditional alert triggering |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| static const WiFiDetectorEntry wifiDetectors[] = { | ||
| { detectSsidFormat, DET_SSID_FORMAT }, | ||
| { detectSsidKeyword, DET_SSID_KEYWORD }, | ||
| { detectWifiMacOui, DET_MAC_OUI }, | ||
| }; | ||
| static const uint8_t WIFI_DETECTOR_COUNT = | ||
| sizeof(wifiDetectors) / sizeof(wifiDetectors[0]); | ||
|
|
||
| static const BLEDetectorEntry bleDetectors[] = { | ||
| { detectBleName, DET_BLE_NAME }, | ||
| { detectRavenCustomUuid, DET_RAVEN_CUSTOM_UUID }, | ||
| { detectRavenStdUuid, DET_RAVEN_STD_UUID }, | ||
| { detectBleMacOui, DET_MAC_OUI }, | ||
| }; | ||
| static const uint8_t BLE_DETECTOR_COUNT = |
There was a problem hiding this comment.
The static arrays wifiDetectors and bleDetectors are defined at file scope in a header file. While this works for the current single-translation-unit Arduino sketch, it could cause multiple definition linker errors if this header is included from multiple .cpp files in the future. Consider either: (1) moving these to a .cpp file with extern declarations in the header, (2) making them inline constexpr in C++17, or (3) wrapping them in an inline namespace or anonymous namespace inside inline function scope.
| static const WiFiDetectorEntry wifiDetectors[] = { | |
| { detectSsidFormat, DET_SSID_FORMAT }, | |
| { detectSsidKeyword, DET_SSID_KEYWORD }, | |
| { detectWifiMacOui, DET_MAC_OUI }, | |
| }; | |
| static const uint8_t WIFI_DETECTOR_COUNT = | |
| sizeof(wifiDetectors) / sizeof(wifiDetectors[0]); | |
| static const BLEDetectorEntry bleDetectors[] = { | |
| { detectBleName, DET_BLE_NAME }, | |
| { detectRavenCustomUuid, DET_RAVEN_CUSTOM_UUID }, | |
| { detectRavenStdUuid, DET_RAVEN_STD_UUID }, | |
| { detectBleMacOui, DET_MAC_OUI }, | |
| }; | |
| static const uint8_t BLE_DETECTOR_COUNT = | |
| inline const WiFiDetectorEntry wifiDetectors[] = { | |
| { detectSsidFormat, DET_SSID_FORMAT }, | |
| { detectSsidKeyword, DET_SSID_KEYWORD }, | |
| { detectWifiMacOui, DET_MAC_OUI }, | |
| }; | |
| inline constexpr uint8_t WIFI_DETECTOR_COUNT = | |
| sizeof(wifiDetectors) / sizeof(wifiDetectors[0]); | |
| inline const BLEDetectorEntry bleDetectors[] = { | |
| { detectBleName, DET_BLE_NAME }, | |
| { detectRavenCustomUuid, DET_RAVEN_CUSTOM_UUID }, | |
| { detectRavenStdUuid, DET_RAVEN_STD_UUID }, | |
| { detectBleMacOui, DET_MAC_OUI }, | |
| }; | |
| inline constexpr uint8_t BLE_DETECTOR_COUNT = |
| uint32_t lastSeenMs; | ||
| uint8_t maxCertainty; | ||
| DeviceState state; | ||
| // 6 + 4 + 4 + 1 + 1 = 16 bytes per slot |
There was a problem hiding this comment.
The comment claims the struct is 16 bytes, but due to structure padding on 32-bit ARM (ESP32), this struct will actually be 20 bytes. The uint32_t fields require 4-byte alignment, so 2 bytes of padding will be inserted after the mac array. Consider either: (1) updating the comment to reflect the actual size (20 bytes), (2) using attribute((packed)) if the smaller size is critical, or (3) reordering fields to minimize padding (e.g., placing uint32_t fields first).
| // 6 + 4 + 4 + 1 + 1 = 16 bytes per slot | |
| // Layout on 32-bit ESP32: 6 (mac) + 2 (padding) + 4 + 4 + 1 + 1 = 18 bytes, rounded up to 20 bytes per slot |
|
|
||
| ## Build System | ||
|
|
||
| This is a pure **Arduino IDE** project (no CMake, PlatformIO, or Makefile). Each hardware variant is a self-contained Arduino sketch that opens directly in the IDE. |
There was a problem hiding this comment.
The documentation states "This is a pure Arduino IDE project (no CMake, PlatformIO, or Makefile)" but this PR adds a Makefile. This is contradictory. The statement should be updated to acknowledge that a Makefile wrapper around arduino-cli is now available, or the phrasing should be clarified to indicate the project doesn't require these tools but now supports optional Makefile-based workflows.
| This is a pure **Arduino IDE** project (no CMake, PlatformIO, or Makefile). Each hardware variant is a self-contained Arduino sketch that opens directly in the IDE. | |
| This project is designed to be built with the **Arduino IDE** or `arduino-cli`; no external build system like CMake or PlatformIO is required. An optional Makefile wrapper around `arduino-cli` is available for CLI-based workflows. Each hardware variant is a self-contained Arduino sketch that opens directly in the IDE. |
| public: | ||
| static const uint8_t MAX_WIFI_CHANNEL = 13; | ||
| static const uint16_t CHANNEL_SWITCH_MS = 500; | ||
| static const uint16_t CHANNEL_SWITCH_MS = 1000; |
There was a problem hiding this comment.
The CHANNEL_SWITCH_MS value changed from 500ms to 1000ms, doubling the WiFi channel hop interval. While CLAUDE.md line 93 documents this as "1 second", this behavioral change should be noted in the PR description. Slower channel switching reduces scan coverage but may improve detection reliability. Consider whether this change was intentional or accidental, and if intentional, document the rationale.
| DOCTEST_URL := https://raw.githubusercontent.com/doctest/doctest/v2.4.11/doctest/doctest.h | ||
|
|
||
| .PHONY: test test-verbose fetch-doctest | ||
|
|
||
| fetch-doctest: test/doctest.h | ||
|
|
||
| test/doctest.h: | ||
| @echo "Fetching doctest.h …" | ||
| curl -sL -o $@ $(DOCTEST_URL) |
There was a problem hiding this comment.
The fetch-doctest rule downloads doctest.h directly from raw.githubusercontent.com using curl and a mutable tag URL without any checksum or signature verification, which creates a supply-chain risk if the upstream repository or delivery channel is compromised. Because this header is compiled and executed as part of the test binary, a malicious upstream change could execute arbitrary code in your CI or developer environment and exfiltrate secrets. To mitigate this, pin the dependency to an immutable identifier (e.g., commit hash or vendored copy) and add integrity verification (such as a known checksum) before using the downloaded file.
Summary
Test plan
make testand verify all tests passmake build VARIANT=m5stickand verify successful compilationmake helpdisplays available targets🤖 Generated with Claude Code