-
Notifications
You must be signed in to change notification settings - Fork 1
MAKE F-CORE LORA GOODER FOR THE GOOD OF HUMANITY !! ! ! ! !! #347
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
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 refactors the LoRa communication subsystem into a layered architecture with three distinct layers: Link (CLoraLink), Network (CLoraRouter), and Application (CLoraTenant). This replaces the previous tightly-coupled design that split TX and RX functionality across two separate tenants.
Key changes:
- Introduces standardized packet framing with a handler-based routing system
- Consolidates LoRa TX/RX into a single tenant with state-machine-based behavior
- Adds GROUND state support to the state machine for ground station operations
Reviewed changes
Copilot reviewed 31 out of 32 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/f_core/radio/c_lora_link.cpp | Link layer implementation handling low-level LoRa TX/RX with standardized framing |
| lib/f_core/radio/c_lora_link.h | Link layer interface defining LaunchLoraFrame structure and RX queue |
| lib/f_core/radio/c_lora_router.cpp | Network layer routing received frames to registered port handlers |
| lib/f_core/radio/c_lora_router.h | Network layer interface with handler registration |
| lib/f_core/radio/c_lora_tenant.cpp | Unified application layer tenant with state-based TX/RX logic |
| lib/f_core/radio/c_lora_tenant.h | Tenant interface extending state machine for flight phases |
| lib/f_core/radio/c_lora_frame_handler.h | Interface for port-specific frame handlers |
| lib/f_core/radio/c_lora.cpp | Hardware layer with renamed async methods and userData support |
| lib/f_core/radio/c_lora.h | Hardware layer interface updates |
| include/f_core/state_machine/c_pad_flight_landing_state_machine.h | Added GROUND state and SetToGround() method |
| lib/f_core/Kconfig | Added F_CORE_STATE_MACHINE dependency for radio module |
| app/ground/receiver_module/* | Updated to use new unified LoRa tenant |
| app/backplane/radio_module/* | Updated to use new unified LoRa tenant |
| app/*/core.conf | Suppressed LoRa driver logs to error level only |
| app/backplane/sensor_module/.idea/vcs.xml | IDE configuration changes |
Files not reviewed (1)
- app/backplane/sensor_module/.idea/vcs.xml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
… size checks, fix kconfig for f-core
| * Handle an incoming radio frame. | ||
| * @param frame The received radio frame | ||
| */ | ||
| virtual void HandleFrame(const LaunchLoraFrame& frame) = 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.
can handlers be able to receive the RSSI and SNR? clora receives it anyways and i think it could be nice to know for things like
GS sends packet asking for like Statistics
FS responds with 'Yes I heard you, RSSI=X, SNR=Y, BAT=Z
which is helpful to know both for peace of mind.
and (prolly more importantly) if we ever do link negotiation (like what payload will do to get the most BW out of the radios for faster images) thats good to know
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.
Ah yes this is a mistake that I forgot to do
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.
Done in #366 since no subclasses for frame handler were implemented in this PR, and making the change here would just cause a mess <3
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.
no cornflakes
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.
Less cornflakes*
Description
Design a subsystem for handling LoRa better by treating things like networking layers and making things significantly more maintainable by eliminating two tightly coupled tenants that had to coordinate using the LoRa device for TX and RX and centralizes it into a single tenant. Have packet framing standardized into a single class. Improved maintainability for handling received lora packets by not just having an if else block with hardcoded ports.
LoraLink
Link layer. Directly interfaces with the hardware. Keeps our lora packet framing standardized instead of it being a wild west across tenants, and ensures that only one place needs to be changed if we change how things are framed. It is still [2 bytes for the port] [remainder for payload] at this time.
By default it enables asynchronous receives, and only disables when the synchronous transmit function is called. Upon receiving a packet, it passes the data, rssi and snr to a queue. It is up to the layers above the link to call the receive function to take items out of the queue and process them!
LoraRouter
Network layer. References the link. Decides where packets should go to be handled. This is done by registering classes, that implement the CLoraFrameHandler interface, to a port. To reduce the scope of this PR, the handlers will be implemented in another PR. This means that no module can currently handle the packets it receives, but restoring functionality should be trivial with this new design. Router has a PollOnce function for grabbing a packet from the receive queue in LoraLink and passing it to the necessary handler
LoraTenant
App layer. Owns both the link and the router. Replaces CLoraReceiveTenant and CLoraTransmitTenant with a unified tenant. State machine to control when to transmit and receive (and how long to wait for a new packet). Basically polls two queues, with the tx call checking the lora transmit queue and calling the send function in LoraLink if something is present. Rx call, calls the PollOnce function in LoraRouter to check if a packet is available and then routing the packet to the handler.
Type of change
How Has This Been Tested?
Ran radio mod and receiver mod, and confirmed both can send and receive data to each other
Checklist: