SNS - LP BMS - #99
Conversation
…laj/hype-28-implement-accelerometers
…laj/hype-28-implement-accelerometers
…laj/hype-28-implement-accelerometers
with correct addresses and manual channel changing
davidbeechey
left a comment
davidbeechey
left a comment
There was a problem hiding this comment.
Nice! A few things to look at, plus making clippy and the formatter happy. Also could you merge main back into this branch, you seem to have included a lot of accel code? Thanks!
| .send(CanMessage::MeasurementReading(MeasurementReading::new( | ||
| CanData::F32(battery_data.voltage), | ||
| board, | ||
| measurement_id, |
There was a problem hiding this comment.
The measurement ID should be unique for each "thing" we're measuring, so all of these will need their own. You can add them to /config/pods.yaml and hard-code them in here using the MeasurementId. Otherwise it'll be hellish to pass into this task (and we'll probs only have one LP BMS so don't need to worry about that)
| /// Driver for the TinyBMS s516 30A Battery Management System using CAN. | ||
| /// Used to monitor battery status and health. |
There was a problem hiding this comment.
Clippy is complaining about these comments, so either put them somewhere else in the file or change them to // instead of /// (probably the former)
| impl<'a, T: HypedCanTx + HypedCanRx> Bms<'a, T> { | ||
| pub fn new(can: &'a mut T) -> Self { | ||
| Bms { can } | ||
| } | ||
|
|
||
| pub fn read_battery_data(&mut self) -> Result<BatteryData, CanError> { | ||
| let voltage = self.read_voltage()?; | ||
| let current = self.read_current()?; | ||
| let max_cell_mv = self.read_max_cell_voltage()?; | ||
| let min_cell_mv = self.read_min_cell_voltage()?; | ||
| let temperatures_c = self.read_temperatures()?; | ||
| let cell_voltages_mv = self.read_cell_voltages()?; | ||
|
|
||
| Ok(BatteryData { | ||
| voltage, | ||
| current, | ||
| max_cell_mv, | ||
| min_cell_mv, | ||
| temperatures_c, | ||
| cell_voltages_mv, | ||
| }) | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
Any reason this is in a separate impl block to the block above? Looks like they're the same
|
|
||
| impl<'a, T: HypedCanTx + HypedCanRx> Bms<'a, T> { | ||
| const NODE_ID: u8 = 0x01; | ||
| const REQUEST_ID: u32 = 0x400 | Self::NODE_ID as u32; |
There was a problem hiding this comment.
What is Self::NODE_ID?
| const RESPONSE_ID: u32 = 0x500 | Self::NODE_ID as u32; | ||
|
|
||
| fn send_simple_request(&mut self, cmd: u8) -> Result<(), CanError> { | ||
| let frame = HypedCanFrame::new(Self::REQUEST_ID, [cmd, 0, 0, 0, 0, 0, 0, 0]); |
There was a problem hiding this comment.
likewise what is Self::REQUEST_ID?
3 participants
No description provided.