light_handler #24
No reviewers
Labels
No labels
bug
duplicate
enhancement
help wanted
invalid
priority
high
priority
low
priority
medium
question
wontfix
No milestone
No project
No assignees
2 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
TTK4145/elevator!24
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "light_handler"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
326f2f6cbe8a11ff72fbWIP: light_handlerto light_handlerA couple of things. I am sorry.
@ -9,6 +9,8 @@ use std::time::Duration;use tokio::net::UdpSocket;use tokio::sync::mpsc;pub const MIN_TX_PUBLISHING_RATE: Duration = Duration::from_millis(250);Please write doccomment.
@ -11,3 +11,3 @@// Waiting for recieved information on the `floor_sensor_rx` from the elevatorlet recieved_floor: u8 = floor_rx.recv().unwrap();trace!("Recieved from floor controller {:?}", recieved_floor);info!("Recieved from floor controller {:?}", recieved_floor);I don't think this should be trace
@ -0,0 +8,4 @@use log::{debug, error, info, trace};use std::collections::HashMap;use strum::IntoEnumIterator;use tokio::sync::Notify; // 0.17.1Why the 0.17.1 comment?
@ -0,0 +47,4 @@for call in &total_calls {let should_be_on ={ call.should_light_be_on(system_state.read().unwrap().elevators.clone()) };if should_be_on {This is very nested and a bit hard to follow.
Can I suggest doing some inverted logic?
e.g.
done this
not enough brain capacity to do this at the moment
Fuck it, we ball
@ -0,0 +33,4 @@for floor in Destination::iter() {lights.insert(Lights::Cab(floor), LightState::Off);// NOTE: not most modular if one wants 5 floors...Can I suggest implementing
Destination::MAXandDestination::MIN?added
@ -0,0 +34,4 @@lights.insert(Lights::Cab(floor), LightState::Off);// NOTE: not most modular if one wants 5 floors...if floor != Destination::Floor1 {Both of these if statements could be one match.
eh? how?
Fuck it, we ball
@ -0,0 +43,4 @@}// Check if lights should be onlet total_calls = { system_state.clone().read().unwrap().calls.calls.clone() };This is not a total of the calls, this is just a copy of the calls?
changed to calls
@ -0,0 +77,4 @@}// Apply it to the consolefor (lights, light_state) in lights {You get
lightsfromlights? I know what is happening, but the variable name is confusing.it was ment to be light
@ -0,0 +78,4 @@// Apply it to the consolefor (lights, light_state) in lights {let (floor, call_type) = match lights {I really struggle to follow this match statement, I am sorry...
its okay
added some comments? maybe helping
recomended: sleep
Fuck it, we ball
@ -0,0 +46,4 @@let total_calls = { system_state.clone().read().unwrap().calls.calls.clone() };for call in &total_calls {let should_be_on ={ call.should_light_be_on(system_state.read().unwrap().elevators.clone()) };Why are we cloning so much, can this not take a reference?
you showed me one example! there was clone, and now i am forever cloning
and as logic is now these get as ref functions does not work. one gets my own elevator, another gets elevator but needs id
Pain, but we are on desktop, so whatever.
@ -0,0 +103,4 @@};match floor {Some(t) => {// NOTE: Remove this line of code is something does not work.Consider removing this note, it should work now, right?
considered and done
@ -0,0 +146,4 @@door_notification: Arc<Notify>,) {loop {let _ = light_controller(Why do we have
let _ =, this function does not return anything?@ -0,0 +162,4 @@//!//! ----------------------------------------------------------------------//! NOTE: Light handler was tested on the actual elevator and therefore has no implemented//! tests at the momentnice!
@ -159,7 +194,7 @@ pub struct StateUpdateSource {}/// Information about a single elevator and it's state#[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize)]Please, re-derive these functions if possible.
removed or added, how you look at it
@ -11,8 +11,9 @@ use std::{sync::OnceLock,time::{Duration, SystemTime},};use strum_macros::EnumIter; // 0.17.1Again, why the version number?
removed
Fuck it, we ball