obstructed #18

Merged
Athamantis merged 16 commits from obstructed into main 2026-02-24 09:38:29 +00:00
Owner
No description provided.
something
All checks were successful
/ build_and_test (push) Successful in 23s
4a5ce74a8a
add the elevator driver example in main
All checks were successful
/ build_and_test (push) Successful in 33s
aa7d0ea807
add get obstructed function in the state controller
Some checks failed
/ build_and_test (push) Failing after 22s
077598e840
Fix the errors for the obstruction_handler
All checks were successful
/ build_and_test (push) Successful in 35s
5ac87a96c5
sebgab requested changes 2026-02-12 15:13:43 +00:00
Dismissed
sebgab left a comment
Owner

Mostly good, most comments are nitpicks.

Mostly good, most comments are nitpicks.
src/main.rs Outdated
@ -16,6 +16,14 @@ use system_state_controller::SystemState;
use crate::system_state_controller::Destination;
use std::thread::*;
Owner

Wildcard imports are frowned upon and should not be used.
For more information see: https://effective-rust.com/wildcard.html

Wildcard imports are frowned upon and should not be used. For more information see: https://effective-rust.com/wildcard.html
Author
Owner

this is from the elevator driver example, modified to use std:🧵:spawn;

this is from the elevator driver example, modified to use std::thread::spawn;
sebgab marked this conversation as resolved
src/main.rs Outdated
@ -19,0 +22,4 @@
use crossbeam_channel as cbc;
use driver_rust::elevio;
use driver_rust::elevio::elev as e;
Owner

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...

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...
Author
Owner

copied from example from elevator driver
modified e -> elev

copied from example from elevator driver modified e -> elev
Athamantis marked this conversation as resolved
src/main.rs Outdated
@ -75,4 +83,76 @@ async fn main() {
let _ = rd_handl.await;
println!("Hello, world!");
let elev_num_floors = 4;
Owner

Code below this point will never execute as we are awaiting tasks above that will never return.

Code below this point will never execute as we are awaiting tasks above that will never return.
Author
Owner

i have now moved it

i have now moved it
Athamantis marked this conversation as resolved
src/main.rs Outdated
@ -78,0 +119,4 @@
elevator.motor_direction(dirn);
}
loop {
Owner

What is the purpose of this loop?

What is the purpose of this loop?
Author
Owner

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 no idea, i just copied the example from the driver https://github.com/TTK4145/driver-rust/blob/master/src/main.rs
Author
Owner

i have now deleted it. yay!

i have now deleted it. yay!
Athamantis marked this conversation as resolved
@ -121,6 +121,33 @@ impl ElevatorInfo {
}
}
/// Takes in a boolean for if the elevator is obstructed or not.
Owner

Doccomment does not describe what the function does, just the input parameter.

Doccomment does not describe what the function does, just the input parameter.
Author
Owner

comments are updated

comments are updated
Owner

The function does not take in a boolean.

The function does not take in a boolean.
Author
Owner

now it is fixed

now it is fixed
Athamantis marked this conversation as resolved
@ -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 door
Owner

This is a great comment! Make this the function's doccomment. :)

This is a great comment! Make this the function's doccomment. :)
Author
Owner

thanks, deleted and written something else....................

thanks, deleted and written something else....................
sebgab marked this conversation as resolved
@ -124,0 +137,4 @@
}
}
None => {
// NOTE: a contition that never should happen, therefore not doing
Owner

contition that never should happen

This is incorrect.

This condition will occur if we call the function with a None value when the elevator is not obstructed.

> contition that never should happen This is incorrect. This condition will occur if we call the function with a `None` value when the elevator is not obstructed.
Author
Owner

comments are updated

comments are updated
Athamantis marked this conversation as resolved
@ -0,0 +1,24 @@
use crate::SystemState;
use crossbeam_channel::{self as cbc, Receiver};
use driver_rust::elevio::poll::obstruction;
use std::sync::*;
Owner

Do not use wildcard imports.

Do not use wildcard imports.
Author
Owner

i have now updated it

i have now updated it
Athamantis marked this conversation as resolved
@ -0,0 +5,4 @@
use super::ObstructionKind;
async fn obstruction_handler(state: Arc<RwLock<SystemState>>, obstruction_rx: Receiver<bool>) -> ! {
Owner

Missing doccomment.

Additionally you should spawn this task from main. Currently it is not used.

Missing doccomment. Additionally you should spawn this task from main. Currently it is not used.
Author
Owner

i have now added doc comments
and spawned the function in main

i have now added doc comments and spawned the function in main
Athamantis marked this conversation as resolved
@ -0,0 +7,4 @@
async fn obstruction_handler(state: Arc<RwLock<SystemState>>, obstruction_rx: Receiver<bool>) -> ! {
loop {
let obstruct = obstruction_rx.recv().unwrap();
Owner

When does this receive happen?

Does this happen when something changes? Does it happen at an interval?

When does this receive happen? Does this happen when something changes? Does it happen at an interval?
Author
Owner

I am not sure. This is copied from the driver.... and i do not understand the driver

I am not sure. This is copied from the driver.... and i do not understand the driver
Owner

Based on line 81 in main where you set poll_period I believe you get a value every 25ms.

Please confirm that the logic will work with this update behaviour.

Based on line 81 in main where you set `poll_period` I believe you get a value every 25ms. Please confirm that the logic will work with this update behaviour.
Author
Owner

no i do not think it will change anything

no i do not think it will change anything
Athamantis marked this conversation as resolved
@ -0,0 +9,4 @@
loop {
let obstruct = obstruction_rx.recv().unwrap();
let mut state = state.write().unwrap();
if obstruct {
Owner

We should add some logging here.

We should add some logging here.
Author
Owner

added

added
Athamantis marked this conversation as resolved
@ -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 state
match obstruction_kind {
Owner

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?

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?
Author
Owner

i have writter better documentation yay!

i have writter better documentation yay!
sebgab marked this conversation as resolved
need more comment, but also e -> elev
All checks were successful
/ build_and_test (push) Successful in 35s
b41e2f9ea4
add comments
All checks were successful
/ build_and_test (push) Successful in 31s
c0afabada1
Add spawning av obstruction handler, and remove the loop in main
Some checks failed
/ build_and_test (push) Failing after 25s
4016add36b
fix comments for function set obstruction
Some checks failed
/ build_and_test (push) Failing after 23s
1be937d6be
fix imports and move som stuff
Some checks failed
/ build_and_test (push) Failing after 23s
eea623b008
fixed import
All checks were successful
/ build_and_test (push) Successful in 33s
da57068bd3
Add tests
Some checks failed
/ build_and_test (push) Failing after 26s
c5f7244789
Fix spacing in test table
Some checks failed
/ build_and_test (push) Failing after 26s
d2c8b658dd
fix the test
All checks were successful
/ build_and_test (push) Successful in 36s
4c11fec144
update comments
All checks were successful
/ build_and_test (push) Successful in 33s
04e6eff66d
sebgab requested changes 2026-02-24 09:20:01 +00:00
Dismissed
sebgab left a comment
Owner

Teeny tiny little thing.

Teeny tiny little thing.
src/main.rs Outdated
@ -18,0 +17,4 @@
use system_state_controller::obstruction_handler;
use std::thread::spawn;
use std::time::*;
Owner

Still a wildcard import here.

Still a wildcard import here.
Author
Owner

fixed

fixed
Athamantis marked this conversation as resolved
Fixed commet
All checks were successful
/ build_and_test (push) Successful in 34s
8f1e4493e5
Author
Owner

Closing #17

Closing #17
sebgab approved these changes 2026-02-24 09:38:05 +00:00
sebgab left a comment
Owner

LGTM

LGTM
Athamantis deleted branch obstructed 2026-02-24 09:39:16 +00:00
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
TTK4145/elevator!18
No description provided.