Skip to content
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.

Sign up for GitHub

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

Stuff Messages to increase throughput #107

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from
Draft

Stuff Messages to increase throughput #107

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2a22612
WIP: Stuff 3 messages into a single packet.
NoahSprenger May 11, 2024
f83838d
Fix: Build Errors
NoahSprenger May 14, 2024
3e3125d
important changes
Jun 1, 2024
4f0656f
fix linter
Jun 1, 2024
cd6282b
fix linting 2
Jun 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 4 additions & 9 deletions boards/communication/src/communication.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,26 +59,26 @@
}

pub struct CanDevice0 {
pub can: Can<
'static,
Can0,
Dependencies<Can0, Gclk0Id, Pin<PA23, Alternate<I>>, Pin<PA22, Alternate<I>>, CAN0>,
Capacities,
>,

Check warning on line 67 in boards/communication/src/communication.rs

View workflow job for this annotation

GitHub Actions / clippy

very complex type used. Consider factoring parts into `type` definitions 

warning: very complex type used. Consider factoring parts into `type` definitions
  --> boards/communication/src/communication.rs:62:14
   |
62 |       pub can: Can<
   |  ______________^
63 | |         'static,
64 | |         Can0,
65 | |         Dependencies<Can0, Gclk0Id, Pin<PA23, Alternate<I>>, Pin<PA22, Alternate<I>>, CAN0>,
66 | |         Capacities,
67 | |     >,
   | |_____^
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#type_complexity
   = note: `#[warn(clippy::type_complexity)]` on by default
line_interrupts: OwnedInterruptSet<Can0, EnabledLine0>,
}

impl CanDevice0 {
pub fn new<S>(
can_rx: Pin<PA23, AlternateI>,
can_tx: Pin<PA22, AlternateI>,
pclk_can: Pclk<Can0, Gclk0Id>,
ahb_clock: AhbClk<Can0>,
peripheral: CAN0,
gclk0: S,
can_memory: &'static mut SharedMemory<Capacities>,
loopback: bool,
) -> (Self, S::Inc)

Check warning on line 81 in boards/communication/src/communication.rs

View workflow job for this annotation

GitHub Actions / clippy

this function has too many arguments (8/7) 

warning: this function has too many arguments (8/7)
  --> boards/communication/src/communication.rs:72:5
   |
72 | /     pub fn new<S>(
73 | |         can_rx: Pin<PA23, AlternateI>,
74 | |         can_tx: Pin<PA22, AlternateI>,
75 | |         pclk_can: Pclk<Can0, Gclk0Id>,
...  |
80 | |         loopback: bool,
81 | |     ) -> (Self, S::Inc)
   | |_______________________^
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#too_many_arguments
   = note: `#[warn(clippy::too_many_arguments)]` on by default
where
S: Source<Id = Gclk0Id> + Increment,
{
Expand Down Expand Up @@ -120,17 +120,15 @@
can.filters_standard()
.push(Filter::Classic {
action: Action::StoreFifo0,
filter: ecan::StandardId::new(messages::node::Node::RecoveryBoard.into())
.unwrap(),
filter: ecan::StandardId::new(messages::node::Node::RecoveryBoard.into()).unwrap(),
mask: ecan::StandardId::ZERO,
})
.unwrap_or_else(|_| panic!("Recovery filter"));

can.filters_standard()
.push(Filter::Classic {
action: Action::StoreFifo1,
filter: ecan::StandardId::new(messages::node::Node::SensorBoard.into())
.unwrap(),
filter: ecan::StandardId::new(messages::node::Node::SensorBoard.into()).unwrap(),
mask: ecan::StandardId::ZERO,
})
.unwrap_or_else(|_| panic!("Sensor filter"));
Expand All @@ -146,8 +144,7 @@
can.filters_standard()
.push(Filter::Classic {
action: Action::StoreFifo0,
filter: ecan::StandardId::new(messages::node::Node::GroundStation.into())
.unwrap(),
filter: ecan::StandardId::new(messages::node::Node::GroundStation.into()).unwrap(),
mask: ecan::StandardId::ZERO,
})
.unwrap_or_else(|_| panic!("Ground Station filter"));
Expand Down Expand Up @@ -266,9 +263,7 @@
mav_sequence: 0,
}
}
pub fn send_message(&mut self, m: Message) -> Result<(), HydraError> {
let payload: Vec<u8, 255> = postcard::to_vec(&m)?;

pub fn send_message(&mut self, payload: Vec<u8, 255>) -> Result<(), HydraError> {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could also create another another send function that takes just a Message type and matches it to the needed size defined by the consts in the messages library. We should have a way to send messages that isn't stuffing.

let mav_header = mavlink::MavHeader {
system_id: 1,
component_id: 1,
Expand Down Expand Up @@ -299,16 +294,16 @@
// Do we need the header?
match msg {
mavlink::uorocketry::MavMessage::POSTCARD_MESSAGE(msg) => {
return Ok(postcard::from_bytes::<Message>(&msg.message)?);

Check warning on line 297 in boards/communication/src/communication.rs

View workflow job for this annotation

GitHub Actions / clippy

unneeded `return` statement 

warning: unneeded `return` statement
   --> boards/communication/src/communication.rs:297:21
    |
297 |                     return Ok(postcard::from_bytes::<Message>(&msg.message)?);
    |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_return
    = note: `#[warn(clippy::needless_return)]` on by default
help: remove `return`
    |
297 -                     return Ok(postcard::from_bytes::<Message>(&msg.message)?);
297 +                     Ok(postcard::from_bytes::<Message>(&msg.message)?)
    |
// weird Ok syntax to coerce to hydra error type.
}
_ => {
herror!(Error, ErrorContext::UnkownPostcardMessage);
return Err(mavlink::error::MessageReadError::Io.into());

Check warning on line 302 in boards/communication/src/communication.rs

View workflow job for this annotation

GitHub Actions / clippy

unneeded `return` statement 

warning: unneeded `return` statement
   --> boards/communication/src/communication.rs:302:21
    |
302 |                     return Err(mavlink::error::MessageReadError::Io.into());
    |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_return
help: remove `return`
    |
302 -                     return Err(mavlink::error::MessageReadError::Io.into());
302 +                     Err(mavlink::error::MessageReadError::Io.into())
    |
}
}
} else {
return Err(mavlink::error::MessageReadError::Io.into());

Check warning on line 306 in boards/communication/src/communication.rs

View workflow job for this annotation

GitHub Actions / clippy

unneeded `return` statement 

warning: unneeded `return` statement
   --> boards/communication/src/communication.rs:306:13
    |
306 |             return Err(mavlink::error::MessageReadError::Io.into());
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_return
help: remove `return`
    |
306 -             return Err(mavlink::error::MessageReadError::Io.into());
306 +             Err(mavlink::error::MessageReadError::Io.into())
    |
}
}
}
Expand Down
148 changes: 56 additions & 92 deletions boards/communication/src/data_manager.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,44 +1,25 @@
use common_arm::HydraError;
use heapless::{HistoryBuffer, Vec};
use messages::command::RadioRate;
use messages::state::StateData;
use messages::Message;
use messages::{MAX_COMMAND_SIZE, MAX_HEALTH_SIZE, MAX_LOG_SIZE, MAX_SENSOR_SIZE, MAX_STATE_SIZE};
use postcard;

Check warning on line 7 in boards/communication/src/data_manager.rs

View workflow job for this annotation

GitHub Actions / clippy

this import is redundant 

warning: this import is redundant
 --> boards/communication/src/data_manager.rs:7:1
  |
7 | use postcard;
  | ^^^^^^^^^^^^^ help: remove it entirely
  |
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#single_component_path_imports
  = note: `#[warn(clippy::single_component_path_imports)]` on by default

const MAX_RADIO_MSG: usize = 255;

#[derive(Clone)]
pub struct DataManager {
pub air: Option<Message>,
pub ekf_nav_1: Option<Message>,
pub ekf_nav_2: Option<Message>,
pub ekf_nav_acc: Option<Message>,
pub ekf_quat: Option<Message>,
pub imu_1: Option<Message>,
pub imu_2: Option<Message>,
pub utc_time: Option<Message>,
pub gps_vel: Option<Message>,
pub gps_vel_acc: Option<Message>,
pub gps_pos_1: Option<Message>,
pub gps_pos_2: Option<Message>,
pub gps_pos_acc: Option<Message>,
pub state: Option<StateData>,
pub message_queue: HistoryBuffer<Message, 64>,
pub logging_rate: Option<RadioRate>,
pub state: Option<StateData>,
}

impl DataManager {
pub fn new() -> Self {
Self {
air: None,
ekf_nav_1: None,
ekf_nav_2: None,
ekf_nav_acc: None,
ekf_quat: None,
imu_1: None,
imu_2: None,
utc_time: None,
gps_vel: None,
gps_vel_acc: None,
gps_pos_1: None,
gps_pos_2: None,
gps_pos_acc: None,
state: None,
message_queue: HistoryBuffer::new(),
logging_rate: Some(RadioRate::Slow), // start slow.
state: None,
}
}

Expand All @@ -49,77 +30,58 @@
return rate_cln;
}
self.logging_rate = Some(RadioRate::Slow);
return RadioRate::Slow;

Check warning on line 33 in boards/communication/src/data_manager.rs

View workflow job for this annotation

GitHub Actions / clippy

unneeded `return` statement 

warning: unneeded `return` statement
  --> boards/communication/src/data_manager.rs:33:9
   |
33 |         return RadioRate::Slow;
   |         ^^^^^^^^^^^^^^^^^^^^^^
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_return
help: remove `return`
   |
33 -         return RadioRate::Slow;
33 +         RadioRate::Slow
   |
}

/// Do not clone instead take to reduce CPU load.
pub fn take_sensors(&mut self) -> [Option<Message>; 13] {
[
self.air.take(),
self.ekf_nav_1.take(),
self.ekf_nav_2.take(),
self.ekf_nav_acc.take(),
self.ekf_quat.take(),
self.imu_1.take(),
self.imu_2.take(),
self.utc_time.take(),
self.gps_vel.take(),
self.gps_vel_acc.take(),
self.gps_pos_1.take(),
self.gps_pos_2.take(),
self.gps_pos_acc.take(),
]
}

pub fn clone_states(&self) -> [Option<StateData>; 1] {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should still be sending state messages, and I think we do since we don't treat it differently than any other message received (with the exception of commands).

[self.state.clone()]
}
pub fn handle_data(&mut self, data: Message) {
match data.data {
messages::Data::Sensor(ref sensor) => match sensor.data {
messages::sensor::SensorData::EkfNavAcc(_) => {
self.ekf_nav_acc = Some(data);
}
messages::sensor::SensorData::GpsPosAcc(_) => {
self.gps_pos_acc = Some(data);
}
messages::sensor::SensorData::Air(_) => {
self.air = Some(data);
}
messages::sensor::SensorData::EkfNav1(_) => {
self.ekf_nav_1 = Some(data);
pub fn stuff_messages(&mut self) -> Result<Vec<u8, 255>, HydraError> {
let mut bytes: Vec<u8, 255> = Vec::new();
for el in self.message_queue.oldest_ordered() {
match el.data {
messages::Data::Command(_) => {
if bytes.len() + MAX_COMMAND_SIZE <= MAX_RADIO_MSG {
bytes.extend(postcard::to_vec::<messages::Message, MAX_COMMAND_SIZE>(el)?);
} else {
break;
}
}
messages::sensor::SensorData::EkfNav2(_) => {
self.ekf_nav_2 = Some(data);
messages::Data::Health(_) => {
if bytes.len() + MAX_HEALTH_SIZE <= MAX_RADIO_MSG {
bytes.extend(postcard::to_vec::<messages::Message, MAX_HEALTH_SIZE>(el)?);
} else {
break;
}
}
messages::sensor::SensorData::EkfQuat(_) => {
self.ekf_quat = Some(data);
messages::Data::Sensor(_) => {
if bytes.len() + MAX_SENSOR_SIZE <= MAX_RADIO_MSG {
bytes.extend(postcard::to_vec::<messages::Message, MAX_SENSOR_SIZE>(el)?);
} else {
break;
}
}
messages::sensor::SensorData::GpsVel(_) => {
self.gps_vel = Some(data);
messages::Data::State(_) => {
if bytes.len() + MAX_STATE_SIZE <= MAX_RADIO_MSG {
bytes.extend(postcard::to_vec::<messages::Message, MAX_STATE_SIZE>(el)?);
} else {
break;
}
}
messages::sensor::SensorData::GpsVelAcc(_) => {
self.gps_vel_acc = Some(data);
messages::Data::Log(_) => {
if bytes.len() + MAX_LOG_SIZE <= MAX_RADIO_MSG {
bytes.extend(postcard::to_vec::<messages::Message, MAX_LOG_SIZE>(el)?);
} else {
break;
}
}
messages::sensor::SensorData::Imu1(_) => {
self.imu_1 = Some(data);
}
messages::sensor::SensorData::Imu2(_) => {
self.imu_2 = Some(data);
}
messages::sensor::SensorData::UtcTime(_) => {
self.utc_time = Some(data);
}
messages::sensor::SensorData::GpsPos1(_) => {
self.gps_pos_1 = Some(data);
}
messages::sensor::SensorData::GpsPos2(_) => {
self.gps_pos_2 = Some(data);
}
},
messages::Data::State(state) => {
self.state = Some(state.data);
}
}
if bytes.len() > 0 {

Check warning on line 77 in boards/communication/src/data_manager.rs

View workflow job for this annotation

GitHub Actions / clippy

length comparison to zero 

warning: length comparison to zero
  --> boards/communication/src/data_manager.rs:77:12
   |
77 |         if bytes.len() > 0 {
   |            ^^^^^^^^^^^^^^^ help: using `!is_empty` is clearer and more explicit: `!bytes.is_empty()`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#len_zero
   = note: `#[warn(clippy::len_zero)]` on by default
return Ok(bytes);
}
return Err(HydraError::from("No messages to send"));

Check warning on line 80 in boards/communication/src/data_manager.rs

View workflow job for this annotation

GitHub Actions / clippy

unneeded `return` statement 

warning: unneeded `return` statement
  --> boards/communication/src/data_manager.rs:80:9
   |
80 |         return Err(HydraError::from("No messages to send"));
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_return
help: remove `return`
   |
80 -         return Err(HydraError::from("No messages to send"));
80 +         Err(HydraError::from("No messages to send"))
   |
}

pub fn handle_data(&mut self, data: Message) {
match data.data {
messages::Data::Command(command) => match command.data {
messages::command::CommandData::RadioRateChange(command_data) => {
self.logging_rate = Some(command_data.rate);
Expand All @@ -128,7 +90,9 @@
messages::command::CommandData::DeployMain(_) => {}
messages::command::CommandData::PowerDown(_) => {}
},
_ => {}
_ => {
self.message_queue.write(data);
}
}
}
}
Expand Down
42 changes: 20 additions & 22 deletions boards/communication/src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
use hal::prelude::*;
use hal::sercom::{spi, spi::Config, spi::Duplex, spi::Pads, spi::Spi, IoSet1, Sercom4};
use health::HealthMonitorChannelsCommunication;
use heapless::Vec;
use mcan::messageram::SharedMemory;
use messages::command::RadioRate;
use messages::health::Health;
Expand Down Expand Up @@ -57,21 +58,21 @@
#[local]
struct Local {
led: Pin<PA05, PushPullOutput>,
sd_manager: SdManager<
Spi<
Config<
Pads<
hal::pac::SERCOM4,
IoSet1,
Pin<PB15, Alternate<C>>,
Pin<PB12, Alternate<C>>,
Pin<PB13, Alternate<C>>,
>,
>,
Duplex,
>,
Pin<PB14, Output<PushPull>>,
>,

Check warning on line 75 in boards/communication/src/main.rs

View workflow job for this annotation

GitHub Actions / clippy

very complex type used. Consider factoring parts into `type` definitions 

warning: very complex type used. Consider factoring parts into `type` definitions
  --> boards/communication/src/main.rs:61:21
   |
61 |           sd_manager: SdManager<
   |  _____________________^
62 | |             Spi<
63 | |                 Config<
64 | |                     Pads<
...  |
74 | |             Pin<PB14, Output<PushPull>>,
75 | |         >,
   | |_________^
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#type_complexity
}

#[monotonic(binds = SysTick, default = true)]
Expand Down Expand Up @@ -197,7 +198,7 @@
/// Idle task for when no other tasks are running.
#[idle]
fn idle(_: idle::Context) -> ! {
loop {}

Check warning on line 201 in boards/communication/src/main.rs

View workflow job for this annotation

GitHub Actions / clippy

empty `loop {}` wastes CPU cycles 

warning: empty `loop {}` wastes CPU cycles
   --> boards/communication/src/main.rs:201:9
    |
201 |         loop {}
    |         ^^^^^^^
    |
    = help: you should either use `panic!()` or add a call pausing or sleeping the thread to the loop body
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#empty_loop
    = note: `#[warn(clippy::empty_loop)]` on by default
}

/// Handles the CAN0 interrupt.
Expand All @@ -220,18 +221,19 @@
});
}

/// Receives a log message from the custom logger so that it can be sent over the radio.
pub fn queue_gs_message(d: impl Into<Data>) {
let message = Message::new(&monotonics::now(), COM_ID, d.into());

send_gs::spawn(message).ok();
let bytes = postcard::to_vec(&message);
if let Ok(bytes) = bytes {
send_gs::spawn(bytes).ok();
}
}

/**
* Sends a message to the radio over UART.
*/
#[task(capacity = 10, shared = [&em, radio_manager])]
fn send_gs(mut cx: send_gs::Context, m: Message) {
fn send_gs(mut cx: send_gs::Context, m: Vec<u8, 255>) {
cx.shared.radio_manager.lock(|radio_manager| {
cx.shared.em.run(|| {
radio_manager.send_message(m)?;
Expand All @@ -241,16 +243,16 @@
}

#[task(capacity = 10, local = [sd_manager], shared = [&em])]
fn sd_dump(cx: sd_dump::Context, m: Message) {
fn sd_dump(cx: sd_dump::Context, m: Vec<u8, 255>) {
let manager = cx.local.sd_manager;
cx.shared.em.run(|| {
let mut buf: [u8; 255] = [0; 255];
let msg_ser = postcard::to_slice_cobs(&m, &mut buf)?;
if let Some(mut file) = manager.file.take() {
manager.write(&mut file, &msg_ser)?;

Check warning on line 252 in boards/communication/src/main.rs

View workflow job for this annotation

GitHub Actions / clippy

this expression creates a reference which is immediately dereferenced by the compiler 

warning: this expression creates a reference which is immediately dereferenced by the compiler
   --> boards/communication/src/main.rs:252:42
    |
252 |                 manager.write(&mut file, &msg_ser)?;
    |                                          ^^^^^^^^ help: change this to: `msg_ser`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrow
    = note: `#[warn(clippy::needless_borrow)]` on by default
manager.file = Some(file);
} else if let Ok(mut file) = manager.open_file("log.txt") {
manager.write(&mut file, &msg_ser)?;

Check warning on line 255 in boards/communication/src/main.rs

View workflow job for this annotation

GitHub Actions / clippy

this expression creates a reference which is immediately dereferenced by the compiler 

warning: this expression creates a reference which is immediately dereferenced by the compiler
   --> boards/communication/src/main.rs:255:42
    |
255 |                 manager.write(&mut file, &msg_ser)?;
    |                                          ^^^^^^^^ help: change this to: `msg_ser`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrow
manager.file = Some(file);
}
Ok(())
Expand All @@ -262,23 +264,17 @@
*/
#[task(shared = [data_manager, &em])]
fn sensor_send(mut cx: sensor_send::Context) {
let (sensors, logging_rate) = cx
.shared
.data_manager
.lock(|data_manager| (data_manager.take_sensors(), data_manager.get_logging_rate()));
let (stuffed_messages, logging_rate) = cx.shared.data_manager.lock(|data_manager| {
(
data_manager.stuff_messages(),
data_manager.get_logging_rate(),
)
});

cx.shared.em.run(|| {
for msg in sensors {
match msg {
Some(x) => {
spawn!(send_gs, x.clone())?;
spawn!(sd_dump, x)?;
}
None => {
continue;
}
}
}
let bytes = postcard::to_vec(&stuffed_messages?)?;
spawn!(send_gs, bytes.clone())?;
spawn!(sd_dump, bytes)?;
Ok(())
});
match logging_rate {
Expand All @@ -300,7 +296,8 @@
cx.shared.em.run(|| {
if let Some(x) = state_data {
let message = Message::new(&monotonics::now(), COM_ID, State::new(x));
spawn!(send_gs, message)?;
let bytes = postcard::to_vec(&message)?;
spawn!(send_gs, bytes)?;
} // if there is none we still return since we simply don't have data yet.
Ok(())
});
Expand All @@ -321,7 +318,8 @@
Health::new(health_manager.monitor.data.clone(), state),
)
});
spawn!(send_gs, msg)?;
let bytes = postcard::to_vec(&msg)?;
spawn!(send_gs, bytes)?;
spawn_after!(report_health, ExtU64::secs(5))?;
Ok(())
});
Expand Down
Loading