obstructed #18
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!18
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "obstructed"
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?
Mostly good, most comments are nitpicks.
@ -16,6 +16,14 @@ use system_state_controller::SystemState;use crate::system_state_controller::Destination;use std::thread::*;Wildcard imports are frowned upon and should not be used.
For more information see: https://effective-rust.com/wildcard.html
this is from the elevator driver example, modified to use std:🧵:spawn;
@ -19,0 +22,4 @@use crossbeam_channel as cbc;use driver_rust::elevio;use driver_rust::elevio::elev as e;Why is this imported as
e?Single letter variables and things should be avoided.
It looks like it is used a total of five (5) times and saves three (3) whole characters each time...
copied from example from elevator driver
modified e -> elev
@ -75,4 +83,76 @@ async fn main() {let _ = rd_handl.await;println!("Hello, world!");let elev_num_floors = 4;Code below this point will never execute as we are awaiting tasks above that will never return.
i have now moved it
@ -78,0 +119,4 @@elevator.motor_direction(dirn);}loop {What is the purpose of this loop?
i have no idea, i just copied the example from the driver https://github.com/TTK4145/driver-rust/blob/master/src/main.rs
i have now deleted it. yay!
@ -121,6 +121,33 @@ impl ElevatorInfo {}}/// Takes in a boolean for if the elevator is obstructed or not.Doccomment does not describe what the function does, just the input parameter.
comments are updated
The function does not take in a boolean.
now it is fixed
@ -123,1 +123,4 @@/// Takes in a boolean for if the elevator is obstructed or not.pub fn set_obstruction(&mut self, obstruction_kind: Option<ObstructionKind>) -> () {// Checks if the obstruction is true, if so it will set the obstruction kind as doorThis is a great comment! Make this the function's doccomment. :)
thanks, deleted and written something else....................
@ -124,0 +137,4 @@}}None => {// NOTE: a contition that never should happen, therefore not doingThis is incorrect.
This condition will occur if we call the function with a
Nonevalue when the elevator is not obstructed.comments are updated
@ -0,0 +1,24 @@use crate::SystemState;use crossbeam_channel::{self as cbc, Receiver};use driver_rust::elevio::poll::obstruction;use std::sync::*;Do not use wildcard imports.
i have now updated it
@ -0,0 +5,4 @@use super::ObstructionKind;async fn obstruction_handler(state: Arc<RwLock<SystemState>>, obstruction_rx: Receiver<bool>) -> ! {Missing doccomment.
Additionally you should spawn this task from main. Currently it is not used.
i have now added doc comments
and spawned the function in main
@ -0,0 +7,4 @@async fn obstruction_handler(state: Arc<RwLock<SystemState>>, obstruction_rx: Receiver<bool>) -> ! {loop {let obstruct = obstruction_rx.recv().unwrap();When does this receive happen?
Does this happen when something changes? Does it happen at an interval?
I am not sure. This is copied from the driver.... and i do not understand the driver
Based on line 81 in main where you set
poll_periodI believe you get a value every 25ms.Please confirm that the logic will work with this update behaviour.
no i do not think it will change anything
@ -0,0 +9,4 @@loop {let obstruct = obstruction_rx.recv().unwrap();let mut state = state.write().unwrap();if obstruct {We should add some logging here.
added
@ -124,0 +127,4 @@// then else it will check if the current state is obstructed. if so it wil sett the// elevator to idle.// else it will not change the current statematch obstruction_kind {While the logic is solid, I do struggle a bit to follow it.
Could we make some examples (preferably as tests) to demonstrate this functions behaviour?
i have writter better documentation yay!
Teeny tiny little thing.
@ -18,0 +17,4 @@use system_state_controller::obstruction_handler;use std::thread::spawn;use std::time::*;Still a wildcard import here.
fixed
Closing #17
LGTM