Open
Conversation
…ation - Added WiFi channel management to the WiFi class, allowing retrieval of the current channel. - Updated esp_now.h to define functions for setting and getting the WiFi channel. - Modified sim_espnow.cpp to bind UDP sockets to channels dynamically based on the current WiFi channel. - Enhanced ReceiverApp and TransmitterApp to support channel discovery and migration, including callbacks for channel changes. - Implemented channel scanning logic to find active transmitters and manage channel acquisition. - Added unit tests for channel validation, discovery packet construction, and ChannelScanner functionality. - Updated API endpoints to reflect changes in channel management.
… and clarity - Updated NVS store initialization in ReceiverApi to use a more generic namespace. - Modified TransmitterShellCommands to enhance command help descriptions and add new commands for input and rescan functionalities. - Changed command syntax from "list" to "get" for channel and trim commands to align with new command structure. - Added new tests for config help, config reset, and model setting to ensure proper command functionality. - Implemented additional tests for mapping and failsafe commands to verify expected behavior. - Enhanced help command tests to include new commands and ensure comprehensive coverage.
There was a problem hiding this comment.
Pull request overview
Die PR erweitert OpenDriveHub um eine kanalbasierte Discovery-/Presence-Logik (WiFi 1/6/11) inkl. neuer Protokollpakete und Scanner-Abstraktion, integriert das in TX/RX Apps & Shell, und passt Simulator sowie Tests auf das neue Modell an.
Changes:
- Neues Channel-Modul (
odh-channel) mit gemeinsamen Konstanten undChannelScannerfür TX/RX. - Protokoll-Erweiterung um Discovery/Presence/Migration-Pakete sowie Umstellung von “Announce” → “ReceiverPresence”.
- Simulator- und Console-/Native-Tests auf neue Kommandos/Discovery-Mechanik umgestellt.
Reviewed changes
Copilot reviewed 31 out of 31 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| firmware/test/test_native/test_protocol.cpp | Aktualisiert Packet-Tests (ReceiverPresence) und registriert neue Channel/Discovery-Tests im Test-Runner. |
| firmware/test/test_native/test_channel.cpp | Neue Native-Unit-Tests für Channel-Konstanten, Packetgrößen/Checksums und ChannelScanner. |
| firmware/test/test_console/test_help.py | Erweitert Help-Assertions um neue/umbenannte Commands (mapping/failsafe/input/rescan/reboot). |
| firmware/test/test_console/test_commands_tx.py | Passt TX-Console-Tests an neue Subcommands an (trim get, module list, input, rescan, bind help). |
| firmware/test/test_console/test_commands_rx.py | Ergänzt RX-Console-Tests für mapping/failsafe sowie config set vehicle. |
| firmware/test/test_console/test_commands_common.py | Stellt channel list → channel get um und ergänzt Tests für config help/reset und Validierung. |
| firmware/src/transmitter/web/TransmitterApi.cpp | Entfernt radio_channel aus der TX-Web-Config API. |
| firmware/src/transmitter/TransmitterApp.h | Fügt Channel-Discovery State/Methoden und rescan() API hinzu. |
| firmware/src/transmitter/TransmitterApp.cpp | Implementiert Channel-Acquisition, NVS-Persistenz für radio_ch und Rescan-Flow. |
| firmware/src/transmitter/shell/TransmitterShellCommands.cpp | Shell: neue Commands/Help/Reset; Umstellung auf get/list-Semantik; config help/reset/model; rescan/reboot. |
| firmware/src/receiver/web/ReceiverApi.cpp | Umstellung auf Presence-Status (isPresencing) und NVS-Namespace odh. |
| firmware/src/receiver/shell/ReceiverShellCommands.cpp | RX Shell erweitert: config help/reset/model/vehicle, mapping, failsafe, reboot; channel get. |
| firmware/src/receiver/ReceiverApp.h | Ergänzt Channel-Discovery State + Migration/Loss-Handling Methoden. |
| firmware/src/receiver/ReceiverApp.cpp | Implementiert Channel-Discovery, Presence-Tick, Transmitter-Loss-Redisccovery und ChannelMigration Handling. |
| firmware/sim/src/sim_espnow.cpp | Simulator: Mapping Channel → UDP Port, Self-Packet Filter, Channel-Switching via Socket-Rebind. |
| firmware/sim/sim_build.py | Fügt Include-Pfad für neue Shared-Lib odh-channel für Shim-Quellen hinzu. |
| firmware/sim/include/WiFi.h | Ergänzt WiFi.channel() via sim_get_wifi_channel(). |
| firmware/sim/include/esp_now.h | Entfernt TX/RX-Portmodell; deklariert sim_get/set_wifi_channel(). |
| firmware/lib/odh-radio/TransmitterRadioLink.h | RadioLink API erweitert: setChannel, DiscoveryRequest/Response, Migration, Callbacks. |
| firmware/lib/odh-radio/TransmitterRadioLink.cpp | Implementiert Channel-Switching (sim/esp_wifi), Discovery- und Migration-Pakete sowie Presence-Handling. |
| firmware/lib/odh-radio/ReceiverRadioLink.h | ReceiverLink API: setChannel, Presence-Modus, Discovery/Response/Migration Callbacks. |
| firmware/lib/odh-radio/ReceiverRadioLink.cpp | Implementiert Presence, DiscoveryRequest, DiscoveryResponse-/Migration-Handling und Channel-Switching. |
| firmware/lib/odh-radio/IRadioLink.h | Neue Callback-Typen für DiscoveryResponse/Request und ChannelMigration. |
| firmware/lib/odh-protocol/Protocol.h | Neue PacketTypes + Packet-Structs (Discovery*, ReceiverPresence, ChannelMigration) und neue DiscoveredVehicle-Definition. |
| firmware/lib/odh-protocol/FunctionMap.h | Passt Legacy-Wrapper-Signaturen an neuen FunctionMapEntry Typ an. |
| firmware/lib/odh-config/Config.h | Entfernt fest verdrahteten Default-WLAN-Kanal und Announce-Intervall zugunsten Channel-Modul. |
| firmware/lib/odh-channel/library.json | Neues PIO-Lib-Metadatenfile für odh-channel. |
| firmware/lib/odh-channel/ChannelScanner.h | Neuer plattformagnostischer Scanner (Callbacks für setChannel/sendDiscovery/delay). |
| firmware/lib/odh-channel/ChannelScanner.cpp | Implementierung Scan/BestChannel/Response-Latching. |
| firmware/lib/odh-channel/Channel.h | Zentrale Channel-Konstanten, Timing und Sim-Port-Mapping. |
| firmware/data/transmitter/index.html | Entfernt Radio-Channel UI und zugehörige Config-POST/GET Felder. |
Comment on lines
+231
to
+237
| void ReceiverApp::runChannelDiscovery() { | ||
| // Wire up the ChannelScanner callbacks to our radio layer | ||
| ChannelScanner scanner([this](uint8_t ch) -> bool { return _radio.setChannel(ch); }, [this](uint8_t /*ch*/) -> bool { return _radio.sendDiscoveryRequest(); }, [](uint32_t ms) { delay(ms); }); | ||
|
|
||
| // Forward DiscoveryResponse to the scanner | ||
| _radio.onDiscoveryResponse([&scanner](uint8_t ch, int8_t rssi, uint8_t devCount) { scanner.onDiscoveryResponse(ch, rssi, devCount); }); | ||
|
|
There was a problem hiding this comment.
In runChannelDiscovery() wird ein onDiscoveryResponse-Callback registriert, das eine Referenz auf den Stack-ChannelScanner scanner capturt. Diese Registrierung bleibt nach Rückkehr aus der Funktion bestehen (z.B. wenn checkTransmitterLoss() später erneut runChannelDiscovery() aufruft), wodurch eingehende DiscoveryResponse später einen Dangling-Reference/UAF auslösen können. Lösung: nach dem Scan den Callback wieder auf einen langlebigen Handler zurücksetzen (z.B. wieder auf den "activity tracking"-Lambda), oder den Scanner als Member halten bzw. einen RAII-Guard nutzen, der den alten Callback am Ende wiederherstellt.
Comment on lines
+66
to
90
| static void rebindSocket() { | ||
| if (s_sock >= 0) { | ||
| close(s_sock); | ||
| s_sock = -1; | ||
| } | ||
| return nullptr; | ||
| } | ||
|
|
||
| /* ── ESP-NOW API ────────────────────────────────────────────────────────── */ | ||
|
|
||
| int esp_now_init() { | ||
| /* Create a UDP socket bound to the listen port. */ | ||
| s_sock = socket(AF_INET, SOCK_DGRAM, 0); | ||
| if (s_sock < 0) { | ||
| perror("[SIM] socket"); | ||
| return ESP_FAIL; | ||
| return; | ||
| } | ||
|
|
||
| int opt = 1; | ||
| setsockopt(s_sock, SOL_SOCKET, SO_REUSEADDR, &opt, sizeof(opt)); | ||
| setsockopt(s_sock, SOL_SOCKET, SO_REUSEPORT, &opt, sizeof(opt)); | ||
|
|
||
| uint16_t port = odh::channel::channelToSimPort(s_currentChannel); | ||
|
|
||
| struct sockaddr_in addr{}; | ||
| addr.sin_family = AF_INET; | ||
| addr.sin_port = htons(SIM_LISTEN_PORT); | ||
| addr.sin_port = htons(port); | ||
| addr.sin_addr.s_addr = htonl(INADDR_LOOPBACK); | ||
|
|
||
| if (bind(s_sock, reinterpret_cast<sockaddr *>(&addr), sizeof(addr)) < 0) { | ||
| perror("[SIM] bind"); |
There was a problem hiding this comment.
Die Simulation bindet alle Geräte eines Kanals auf denselben UDP-Port (SO_REUSEPORT) und sendet unicast an 127.0.0.1:port. Bei mehreren Prozessen/Sockets auf demselben Port werden UDP-Datagramme typischerweise nur an EINEN Empfänger verteilt (hash/load-balancing), nicht an alle – damit können Pakete beim falschen Prozess landen und dann durch den Self-MAC-Filter verworfen werden (flaky/defekte Sim-Kommunikation). Zudem wird in sim_set_wifi_channel() während recvLoop läuft der Socket geschlossen/neu gebunden, ohne Synchronisation. Bitte auf ein echtes Broadcast/Multicast pro Kanal umstellen (z.B. UDP multicast group je Kanal) oder ein explizites Fan-out/Relay implementieren; und Channel-Switching so gestalten, dass der recv-Thread sauber gestoppt/neu gestartet oder der FD thread-safe getauscht wird.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces a new, platform-agnostic channel scanning and discovery system for OpenDriveHub, refactoring how WiFi channel selection and device discovery are handled. It removes the manual WiFi channel setting from the transmitter UI, adds a new
odh-channellibrary with channel scanning logic, and updates the protocol to support a multi-stage discovery process using new packet types. Several legacy protocol elements and compatibility aliases are removed, and related code is modernized.The most important changes are:
Channel Scanning and Discovery System:
odh-channellibrary (Channel.h,ChannelScanner.h,ChannelScanner.cpp,library.json) that provides shared constants, candidate channel definitions (1, 6, 11), timing parameters, and a platform-agnosticChannelScannerclass for transmitter/receiver discovery and channel selection. ([[1]](https://github.com/peterus/OpenDriveHub/pull/5/files#diff-4d044102101f7bcae9988c7926f5e383c8df230b15c09b54a9cf25953c0e579aR1-R83),[[2]](https://github.com/peterus/OpenDriveHub/pull/5/files#diff-87e89b5d7e9a172318336e06695a6455dfb6fef47ad9ab934034f06117ea86d1R1-R81),[[3]](https://github.com/peterus/OpenDriveHub/pull/5/files#diff-bd318187cc43f37ed6cb6e826df2222b7b86ab04dddc3bbbf7de1da113c28b94R1-R97),[[4]](https://github.com/peterus/OpenDriveHub/pull/5/files#diff-d7e22310954fae837fdf86fbf5dec4f1e87f34a2d9c1a625b506232acb751e2bR1-R8))Protocol.h) for multi-stage discovery:DiscoveryRequestPacket,DiscoveryResponsePacket,ReceiverPresencePacket, andChannelMigrationPacket, along with theDeviceRoleenum andDiscoveredVehiclestruct. ([[1]](https://github.com/peterus/OpenDriveHub/pull/5/files#diff-dbfff5ce784513a67041f5bdafe276424efe6c57d85b486335d72644bda6f7aaL71-R82),[[2]](https://github.com/peterus/OpenDriveHub/pull/5/files#diff-dbfff5ce784513a67041f5bdafe276424efe6c57d85b486335d72644bda6f7aaL161-R225),[[3]](https://github.com/peterus/OpenDriveHub/pull/5/files#diff-dbfff5ce784513a67041f5bdafe276424efe6c57d85b486335d72644bda6f7aaL225-R282))UI and Config Changes:
index.html), reflecting the move to automatic channel scanning. ([[1]](https://github.com/peterus/OpenDriveHub/pull/5/files#diff-7c3f8d0303a54bf02cabad27c470dabf8c88a3b94936e764c1ca220fea90dccaL186-L189),[[2]](https://github.com/peterus/OpenDriveHub/pull/5/files#diff-7c3f8d0303a54bf02cabad27c470dabf8c88a3b94936e764c1ca220fea90dccaL269),[[3]](https://github.com/peterus/OpenDriveHub/pull/5/files#diff-7c3f8d0303a54bf02cabad27c470dabf8c88a3b94936e764c1ca220fea90dccaL348))kRadioWifiChannelconstant fromConfig.h. ([firmware/lib/odh-config/Config.hL41-L43](https://github.com/peterus/OpenDriveHub/pull/5/files#diff-cd8a6e33d63cb9695e45ebe20ed0d4a3bd286414b7e42c59fc7f114c781463d0L41-L43))Protocol and Legacy Cleanup:
Announce,Ack) and legacy compatibility aliases from the protocol header, as well as the correspondingAnnouncePacketstructure. ([[1]](https://github.com/peterus/OpenDriveHub/pull/5/files#diff-dbfff5ce784513a67041f5bdafe276424efe6c57d85b486335d72644bda6f7aaL71-R82),[[2]](https://github.com/peterus/OpenDriveHub/pull/5/files#diff-dbfff5ce784513a67041f5bdafe276424efe6c57d85b486335d72644bda6f7aaL161-R225),[[3]](https://github.com/peterus/OpenDriveHub/pull/5/files#diff-dbfff5ce784513a67041f5bdafe276424efe6c57d85b486335d72644bda6f7aaL225-R282))FunctionMapEntrytype and modernized return values. ([[1]](https://github.com/peterus/OpenDriveHub/pull/5/files#diff-a4305670a786cc8b4ae5d3049e79cafe85d80e17ca7326b26b905c19e7593babL262-R268),[[2]](https://github.com/peterus/OpenDriveHub/pull/5/files#diff-a4305670a786cc8b4ae5d3049e79cafe85d80e17ca7326b26b905c19e7593babL277-R283))These changes lay the groundwork for robust, automatic channel selection and device discovery, simplifying the user experience and improving system reliability.