Skip to content

Commit

Permalink
thermal: remove panic formatting and Debug impls (#1738)
Browse files Browse the repository at this point in the history
Currently, the `thermal` task uses `unwrap()` in a bunch of places,
requiring it to derive `Debug` for a bunch of stuff. This commit changes
those to `unwrap_lite()`, and removes many of the derived `Debug` impls.
On `gimlet-f`, this shaves off about 1K of flash:

before:
```

thermal              flash   23316  23328  32768
                     ram     7820   8192   8192
```

after:

```
thermal              flash   22392  22400  32768
                     ram     7820   8192   8192
```

It would be nice to also remove the derived `Debug` from types like
`SensorId`, but it looks like that's currently used by `validate_api`
--- I wasn't sure if any of the `validate_api` types are used in
control-plane comms, so I didn't touch that yet. May do so in a
follow-up.
  • Loading branch information
hawkw authored Apr 8, 2024
1 parent d72a833 commit abfdfbe
Show file tree
Hide file tree
Showing 4 changed files with 13 additions and 12 deletions.
1 change: 0 additions & 1 deletion task/thermal-api/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,6 @@ impl ThermalProperties {
#[derive(
Copy,
Clone,
Debug,
Eq,
PartialEq,
Serialize,
Expand Down
6 changes: 3 additions & 3 deletions task/thermal/src/bsp/gimlet_bcdef.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use drv_gimlet_seq_api::{PowerState, Sequencer};
use drv_i2c_devices::max31790::Max31790;
use task_sensor_api::SensorId;
use task_thermal_api::ThermalProperties;
use userlib::{task_slot, units::Celsius, TaskId};
use userlib::{task_slot, units::Celsius, TaskId, UnwrapLite};

task_slot!(SEQ, gimlet_seq);

Expand Down Expand Up @@ -94,7 +94,7 @@ bitflags::bitflags! {

impl Bsp {
pub fn fan_control(&self, fan: crate::Fan) -> FanControl<'_> {
FanControl::Max31790(&self.fctrl, fan.0.try_into().unwrap())
FanControl::Max31790(&self.fctrl, fan.0.try_into().unwrap_lite())
}

pub fn for_each_fctrl(&self, mut fctrl: impl FnMut(FanControl<'_>)) {
Expand Down Expand Up @@ -159,7 +159,7 @@ impl Bsp {
pub fn new(i2c_task: TaskId) -> Self {
// Initializes and build a handle to the fan controller IC
let fctrl = Max31790::new(&devices::max31790(i2c_task)[0]);
fctrl.initialize().unwrap();
fctrl.initialize().unwrap_lite();

// Handle for the sequencer task, which we check for power state
let seq = Sequencer::from(SEQ.get_task_id());
Expand Down
8 changes: 4 additions & 4 deletions task/thermal/src/bsp/sidecar_bcd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ pub use drv_sidecar_seq_api::SeqError;
use drv_sidecar_seq_api::{Sequencer, TofinoSeqState, TofinoSequencerPolicy};
use task_sensor_api::SensorId;
use task_thermal_api::ThermalProperties;
use userlib::{task_slot, units::Celsius, TaskId};
use userlib::{task_slot, units::Celsius, TaskId, UnwrapLite};

include!(concat!(env!("OUT_DIR"), "/i2c_config.rs"));
use i2c_config::devices;
Expand Down Expand Up @@ -114,7 +114,7 @@ impl Bsp {
3 => 1,
_ => panic!(),
};
FanControl::Max31790(controller, fan_physical.try_into().unwrap())
FanControl::Max31790(controller, fan_physical.try_into().unwrap_lite())
}

pub fn for_each_fctrl(&self, mut fctrl: impl FnMut(FanControl<'_>)) {
Expand Down Expand Up @@ -162,8 +162,8 @@ impl Bsp {

let fctrl_east = Max31790::new(&devices::max31790_east(i2c_task));
let fctrl_west = Max31790::new(&devices::max31790_west(i2c_task));
fctrl_east.initialize().unwrap();
fctrl_west.initialize().unwrap();
fctrl_east.initialize().unwrap_lite();
fctrl_west.initialize().unwrap_lite();

Self {
seq,
Expand Down
10 changes: 6 additions & 4 deletions task/thermal/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ impl From<usize> for Fan {
task_slot!(I2C, i2c_driver);
task_slot!(SENSOR, sensor);

#[derive(Copy, Clone, Debug, Eq, PartialEq, counters::Count)]
#[derive(Copy, Clone, Eq, PartialEq, counters::Count)]
enum Trace {
#[count(skip)]
None,
Expand Down Expand Up @@ -357,9 +357,9 @@ fn main() -> ! {
runtime: 0,
};
if bsp::USE_CONTROLLER {
server.set_mode_auto().unwrap();
server.set_mode_auto().unwrap_lite();
} else {
server.set_mode_manual(PWMDuty(0)).unwrap();
server.set_mode_manual(PWMDuty(0)).unwrap_lite();
}

//
Expand All @@ -370,7 +370,9 @@ fn main() -> ! {
// which may induce a flight-or-fight reaction for whomever is near the
// fans when they blast off...
//
server.set_watchdog(I2cWatchdog::ThirtySeconds).unwrap();
server
.set_watchdog(I2cWatchdog::ThirtySeconds)
.unwrap_lite();

let mut buffer = [0; idl::INCOMING_SIZE];
loop {
Expand Down

0 comments on commit abfdfbe

Please sign in to comment.