Add: Device manager #2

RaulTrombin · 2024-06-04T19:27:18Z

This PR adds DeviceManager and Device modules, each following the actor pattern.
Each structure has its own handler, allowing usage across multiple and different threads.

Device requests made to the Manager are forwarded directly to the respective Device. Each request provides a Result<Answer, ManagerError>.

Requires merge&bump on ping-rs:

Add : Serialization required for ping-viewer-next ping-rs#52

some cool refs:
https://ryhl.io/blog/actors-with-tokio/
https://theari.dev/blog/tokio-actors/
https://www.oreilly.com/library/view/applied-akka-patterns/9781491934876/ch01.html
https://www.amazon.com/Rust-Web-Programming-hands-applications/dp/1803234695

patrickelectric · 2024-07-17T03:07:55Z

src/device/devices.rs

+            Ping1DRequest::DeviceID => match self.device_id().await {
+                Ok(result) => PingAnswer::PingMessage(bluerobotics_ping::Messages::Ping1D(
+                    bluerobotics_ping::ping1d::Messages::DeviceId(result),
+                )),
+                Err(e) => PingAnswer::PingError(e),
+            },


Can we just use a macro for this ?

it'll require some manipulation on strings, custom input for return type and different handling for methods.
But'll help to make to code more cleanner, can we add as an future upgrade?

patrickelectric · 2024-07-17T03:10:36Z

src/device/manager.rs

+    DeviceNotExist(Uuid),
+    DeviceAlreadyExist(Uuid),
+    DeviceIsStopped(Uuid),
+    Other,


Can we give an string as content to have more information about the error ?

removed this error for now, was used on only one place with an unreachable case.

patrickelectric · 2024-07-17T03:10:51Z

src/device/manager.rs

+};
+use tokio::sync::{mpsc, oneshot};
+use tokio_serial::{SerialPort, SerialPortBuilderExt, SerialStream};
+// use tracing::{error, info};


lost comment

patrickelectric · 2024-07-17T16:00:34Z

src/device/devices.rs

+            PingRequest::Ping1D(device_request) => match &self.device_type {
+                DeviceType::Ping1D(device) => {
+                    let answer = device.handle(device_request).await;
+                    let _ = request.respond_to.send(DeviceActorAnswer { answer });
+                }
+                _ => {
+                    let ping_request = request.request;
+                    let _ = request.respond_to.send(DeviceActorAnswer {
+                        answer: PingAnswer::NotSupported(ping_request),
+                    });
+                }
+            },
+            PingRequest::Ping360(device_request) => match &self.device_type {
+                DeviceType::Ping360(device) => {
+                    let answer = device.handle(device_request).await;
+                    let _ = request.respond_to.send(DeviceActorAnswer { answer });
+                }
+                _ => {
+                    let ping_request = request.request;
+                    let _ = request.respond_to.send(DeviceActorAnswer {
+                        answer: PingAnswer::NotSupported(ping_request),
+                    });
+                }
+            },
+            PingRequest::Common(device_request) => match &self.device_type {
+                DeviceType::Common(device) => {
+                    let answer = device.handle(device_request).await;
+                    let _ = request.respond_to.send(DeviceActorAnswer { answer });
+                }
+                _ => {
+                    let ping_request = request.request;
+                    let _ = request.respond_to.send(DeviceActorAnswer {
+                        answer: PingAnswer::NotSupported(ping_request),
+                    });
+                }
+            },


Suggested change

PingRequest::Ping1D(device_request) => match &self.device_type {

DeviceType::Ping1D(device) => {

let answer = device.handle(device_request).await;

let _ = request.respond_to.send(DeviceActorAnswer { answer });

}

_ => {

let ping_request = request.request;

let _ = request.respond_to.send(DeviceActorAnswer {

answer: PingAnswer::NotSupported(ping_request),

});

}

},

PingRequest::Ping360(device_request) => match &self.device_type {

DeviceType::Ping360(device) => {

let answer = device.handle(device_request).await;

let _ = request.respond_to.send(DeviceActorAnswer { answer });

}

_ => {

let ping_request = request.request;

let _ = request.respond_to.send(DeviceActorAnswer {

answer: PingAnswer::NotSupported(ping_request),

});

}

},

PingRequest::Common(device_request) => match &self.device_type {

DeviceType::Common(device) => {

let answer = device.handle(device_request).await;

let _ = request.respond_to.send(DeviceActorAnswer { answer });

}

_ => {

let ping_request = request.request;

let _ = request.respond_to.send(DeviceActorAnswer {

answer: PingAnswer::NotSupported(ping_request),

});

}

},

PingRequest::Ping1D(device_request) | PingRequest::Ping360(device_request) | PingRequest::Common(device_request) => match &self.device_type {

DeviceType::Ping1D(device) | DeviceType::Ping360(device) | DeviceType::Common(device) => {

let answer = device.handle(device_request).await;

let _ = request.respond_to.send(DeviceActorAnswer { answer });

}

_ => {

let ping_request = request.request;

let _ = request.respond_to.send(DeviceActorAnswer {

answer: PingAnswer::NotSupported(ping_request),

});

}

},

ping @RaulTrombin

Different devices and requests types :-/
Please check below:

error[E0308]: mismatched types --> src/device/devices.rs:20:72 | 19 | match request.request.clone() { | ----------------------- this expression has type `PingRequest` 20 | PingRequest::Ping1D(device_request) | PingRequest::Ping360(device_request) | PingRequest::Common(device_request) => match &self.device_type { | -------------- ^^^^^^^^^^^^^^ expected `Ping1DRequest`, found `Ping360Request` | | | first introduced with type `Ping1DRequest` here | = note: in the same arm, a binding must have the same type in all alternatives error[E0308]: mismatched types --> src/device/devices.rs:20:110 | 19 | match request.request.clone() { | ----------------------- this expression has type `PingRequest` 20 | PingRequest::Ping1D(device_request) | PingRequest::Ping360(device_request) | PingRequest::Common(device_request) => match &self.device_type { | -------------- first introduced with type `Ping1DRequest` here ^^^^^^^^^^^^^^ expected `Ping1DRequest`, found `PingCommonRequest` | = note: in the same arm, a binding must have the same type in all alternatives error[E0308]: mismatched types --> src/device/devices.rs:21:66 | 20 | PingRequest::Ping1D(device_request) | PingRequest::Ping360(device_request) | PingRequest::Common(device_request) => match &self.device_type { | ----------------- this expression has type `&DeviceType` 21 | DeviceType::Ping1D(device) | DeviceType::Ping360(device) | DeviceType::Common(device) => { | ------ ^^^^^^ expected `&Ping1D`, found `&Ping360` | | | first introduced with type `&bluerobotics_ping::device::Ping1D` here | = note: expected reference `&bluerobotics_ping::device::Ping1D` found reference `&bluerobotics_ping::device::Ping360` = note: in the same arm, a binding must have the same type in all alternatives error[E0308]: mismatched types --> src/device/devices.rs:21:95 | 20 | PingRequest::Ping1D(device_request) | PingRequest::Ping360(device_request) | PingRequest::Common(device_request) => match &self.device_type { | ----------------- this expression has type `&DeviceType` 21 | DeviceType::Ping1D(device) | DeviceType::Ping360(device) | DeviceType::Common(device) => { | ------ ^^^^^^ expected `&Ping1D`, found `&Device` | | | first introduced with type `&bluerobotics_ping::device::Ping1D` here | = note: expected reference `&bluerobotics_ping::device::Ping1D` found reference `&bluerobotics_ping::common::Device` = note: in the same arm, a binding must have the same type in all alternatives For more information about this error, try `rustc --explain E0308`. warning: `ping-viewer-next` (bin "ping-viewer-next") generated 1 warning error: could not compile `ping-viewer-next` (bin "ping-viewer-next") due to 4 previous errors; 1 warning emitted

ping @RaulTrombin

patrickelectric · 2024-07-17T16:01:24Z

src/device/devices.rs

+                };
+                device_type
+            }
+            DeviceType::Ping1D(device) => {


We should only deal with common case, the others are already upgraded.

the function was designed to:
If they already belong to the upgrading available class, they'll return quickly;
if not, they'll be repaired in the next step.

If it is not dealt with, we will remove this functionality.

patrickelectric · 2024-07-17T16:17:06Z

src/device/manager.rs

+        let device = match device_selection {
+            DeviceSelection::Common | DeviceSelection::Auto => match port {
+                SourceType::Udp(udp_port) => crate::device::devices::DeviceType::Common(
+                    bluerobotics_ping::common::Device::new(udp_port),
+                ),
+                SourceType::Serial(serial_port) => crate::device::devices::DeviceType::Common(
+                    bluerobotics_ping::common::Device::new(serial_port),
+                ),
+            },
+            DeviceSelection::Ping1D => match port {
+                SourceType::Udp(udp_port) => {
+                    crate::device::devices::DeviceType::Ping1D(Ping1D::new(udp_port))
+                }
+                SourceType::Serial(serial_port) => {
+                    crate::device::devices::DeviceType::Ping1D(Ping1D::new(serial_port))
+                }
+            },
+            DeviceSelection::Ping360 => match port {
+                SourceType::Udp(udp_port) => {
+                    crate::device::devices::DeviceType::Ping360(Ping360::new(udp_port))
+                }
+                SourceType::Serial(serial_port) => {
+                    crate::device::devices::DeviceType::Ping360(Ping360::new(serial_port))
+                }
+            },
+        };


You should match port first and than the device.

RaulTrombin force-pushed the Add_device_mnge branch 7 times, most recently from bc66eda to 2e968e6 Compare July 16, 2024 18:22

RaulTrombin changed the title ~~Add: Device manager [WIP]~~ Add: Device manager Jul 16, 2024

RaulTrombin requested review from patrickelectric and joaoantoniocardoso July 16, 2024 18:34

RaulTrombin marked this pull request as ready for review July 16, 2024 18:35

patrickelectric requested changes Jul 17, 2024

View reviewed changes

RaulTrombin added 5 commits July 18, 2024 11:04

src:device: Add device and device manager modules

1fd48e5

Cargo: Add required modeules for devices module

0b8e906

src: device: Update to use results

ca495fa

src: device: Add logging

a705865

Cargo: Update ping-rs

738dc60

RaulTrombin force-pushed the Add_device_mnge branch from 2e968e6 to 738dc60 Compare July 18, 2024 14:10

RaulTrombin requested a review from patrickelectric July 18, 2024 14:20

RaulTrombin mentioned this pull request Jul 18, 2024

Add : Device manager integrations (RestAPI and Websocket) #4

Merged

1 task

patrickelectric approved these changes Jul 18, 2024

View reviewed changes

patrickelectric merged commit 1a6c538 into bluerobotics:master Jul 18, 2024
1 check passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add: Device manager #2

Add: Device manager #2

RaulTrombin commented Jun 4, 2024 •

edited

Loading

patrickelectric Jul 17, 2024

RaulTrombin Jul 18, 2024 •

edited

Loading

patrickelectric Jul 17, 2024

RaulTrombin Jul 18, 2024 •

edited

Loading

patrickelectric Jul 17, 2024

patrickelectric Jul 17, 2024

patrickelectric Jul 18, 2024

RaulTrombin Jul 18, 2024 •

edited

Loading

patrickelectric Jul 17, 2024

RaulTrombin Jul 18, 2024

patrickelectric Jul 17, 2024

RaulTrombin Jul 18, 2024

Add: Device manager #2

Add: Device manager #2

Conversation

RaulTrombin commented Jun 4, 2024 • edited Loading

Choose a reason for hiding this comment

RaulTrombin Jul 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RaulTrombin Jul 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RaulTrombin Jul 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RaulTrombin commented Jun 4, 2024 •

edited

Loading

RaulTrombin Jul 18, 2024 •

edited

Loading

RaulTrombin Jul 18, 2024 •

edited

Loading

RaulTrombin Jul 18, 2024 •

edited

Loading