-
Notifications
You must be signed in to change notification settings - Fork 1
GIFT EVERYONE EVERYTHING THEY COULD WANT WITH LORA #366
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
…en out in the tenant. Also, allow raw sending too if someone chooses
…ut 0 bytes on receive. Might be a better model to do async regardless...
…ad of doing it in the isr
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR re-introduces GPIO commanding and UDP packet reconstruction from LoRa payloads, and adds two major new features: the ability to schedule telemetry downlinks and remotely change LoRa frequencies. The implementation refactors the LoRa communication architecture to use a router/handler pattern for better modularity.
Key Changes:
- Implemented remote LoRa frequency changing with ACK-based confirmation
- Added telemetry downlink scheduling system based on configurable timers
- Refactored LoRa communication to use link/router/handler architecture for better separation of concerns
Reviewed changes
Copilot reviewed 32 out of 32 changed files in this pull request and generated 16 comments.
Show a summary per file
| File | Description |
|---|---|
lib/f_core/state_machine/c_pad_flight_landing_state_machine.cpp |
Added state entry/exit hooks for better state transition handling |
lib/f_core/radio/frame_handlers/c_lora_freq_request_tenant.cpp |
Implements ground-side frequency change request with timeout and revert logic |
lib/f_core/radio/frame_handlers/c_frequency_change_handler.cpp |
Handles frequency change commands on rocket side with ACK response |
lib/f_core/radio/frame_handlers/c_lora_frame_to_udp_handler.cpp |
Forwards LoRa frames to UDP for telemetry reconstruction |
lib/f_core/radio/c_lora_tenant.cpp |
State machine-aware LoRa tenant coordinating TX/RX operations |
lib/f_core/radio/c_lora_router.cpp |
Routes received LoRa frames to registered port handlers |
lib/f_core/radio/c_lora_link.cpp |
Low-level LoRa link layer handling frame serialization/deserialization |
lib/f_core/radio/c_lora.cpp |
Enhanced to support disabling/re-enabling async RX during frequency changes |
include/f_core/utils/c_soft_timer.h |
Made non-copyable and non-movable for safety |
include/f_core/utils/c_hashmap.h |
Added initializer list constructor, Emplace, and GetPtr methods |
include/f_core/messaging/c_message_port.h |
Changed AvailableSpace return type from int to size_t |
app/ground/receiver_module/src/c_receiver_module.cpp |
Integrated new frequency change tenant and LoRa router |
app/backplane/radio_module/src/c_remote_gpio_handler.cpp |
Re-implemented GPIO commanding via LoRa frames |
app/backplane/radio_module/src/c_downlink_scheduler_tenant.cpp |
Manages scheduled telemetry downlinks based on flight state and timers |
app/backplane/radio_module/src/c_radio_module.cpp |
Integrated new handlers and downlink scheduler |
boards/arm/radio_module/radio_module.dts |
Updated clock configuration for radio module |
data/autocoder_inputs/network_defs.yaml |
Added frequency change command/response ports |
Comments suppressed due to low confidence (1)
lib/f_core/radio/c_lora_tenant.cpp:11
- The Startup function is empty but still declared as an override. If there's no startup logic needed, this function can be removed entirely since the base class likely provides a default implementation or makes it optional.
void CLoraTenant::Startup() {
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| CLatestMessagePort<LaunchLoraFrame> gnssTelemetryMessagePort; | ||
|
|
||
|
|
||
| // Build the hashamp here for passing into downlink scheduler tenant |
Copilot
AI
Dec 24, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a typo in the comment. "hashamp" should be "hashmap".
| // Build the hashamp here for passing into downlink scheduler tenant | |
| // Build the hashmap here for passing into downlink scheduler tenant |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
true
include/f_core/state_machine/c_pad_flight_landing_state_machine.h
Outdated
Show resolved
Hide resolved
| if (ret == -ENOMSG) { | ||
| LOG_WRN_ONCE("Downlink queue full, clearing and retrying"); | ||
| downlinkMessagePort.Clear(); | ||
| return downlinkMessagePort.Send(frame, K_NO_WAIT) == 0; |
Copilot
AI
Dec 24, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the downlink queue is full and clearing fails or if Send fails after clearing, the function returns false but logs with LOG_WRN_ONCE. Using LOG_WRN_ONCE means subsequent failures won't be logged, which could hide recurring issues. Consider using regular LOG_WRN or implementing a counter to track how often this occurs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think i agree with this. this should just be log_wrn since if im sitting there working i will want to know every time the queue is full
include/f_core/radio/frame_handlers/c_lora_freq_request_tenant.h
Outdated
Show resolved
Hide resolved
| if (ret == -ENOMSG) { | ||
| LOG_WRN_ONCE("Downlink queue full, clearing and retrying"); | ||
| downlinkMessagePort.Clear(); | ||
| return downlinkMessagePort.Send(frame, K_NO_WAIT) == 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think i agree with this. this should just be log_wrn since if im sitting there working i will want to know every time the queue is full
|
|
||
| // Validate frequency range before sending command | ||
| const bool within915 = (freqHz >= 902'000'000u && freqHz <= 928'000'000u); | ||
| const bool within433 = (freqHz >= 410'000'000u && freqHz <= 525'000'000u); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nitpick but im p sure the 433 band ends at 450 mhz
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technically SX1262 go from like 150 to 960 mhz and the specific range depends on the circuitry its tuned for. I've seen some specs reference 493 and others reference 510. I will drop to 450 though because of laws
| CLatestMessagePort<LaunchLoraFrame> gnssTelemetryMessagePort; | ||
|
|
||
|
|
||
| // Build the hashamp here for passing into downlink scheduler tenant |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
true
| CLoraFrameHandler* handler = handlers.Get(rxFrame.Frame.Port).value_or(defaultHandler); | ||
|
|
||
| if (handler != nullptr) { | ||
| handler->HandleFrame(frame); | ||
| handler->HandleFrame(rxFrame); | ||
| } else { | ||
| LOG_WRN("no handler for port %u", frame.Port); | ||
| LOG_WRN("no handler for port %u", rxFrame.Frame.Port); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can this codepath be simplified since handler shouldnt be nullptr ever anymore?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
? It can still be nullptr because defaultHandler can be nullptr
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See line 38 of c_lora_router.h, this is in the event you dont want to install a default handler
Description
Note: #347 should be looked at first and merged. This PR branches off that one
Setting merge to #347 now so the diff doesn't look as bad :)
Re-introduces handling for commanding GPIOs and reconstructing UDP packets from LoRa payloads. But, more importantly... WE CAN SCHEDULE HOW OFTEN TELEMETRY IS DOWNLINKED AND REMOTELY CHANGE LORA FREQUENCIES!!!! MERRY CHRISTMAS!
Type of change
How Has This Been Tested?
Tested commanding GPIOs and changing frequencies (changing frequencies will get more refinement soon™️, but a good proof-of-concept has been laid out now). Receiver mod can now forward downlinked telem over Ethernet again too.
Checklist: