Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow using stdio as a 'Device' #86

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Conversation

usbalbin
Copy link

A semi hacky POC for #85

(Yes, I have a low res screen :) )

[albin@macen serial-monitor-rust]$ ping 1.1.1.1 | cargo r

image

image

BTW, any ideas as to why the data does not show up until "number of plots" is increased to 2?

split_data
.iter()
payload
.split(&[':', ',', '=', ' ', '\t'])
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was just to get the output of ping to look nice in the screenshot. I am happy to revert this if you want

src/stdio.rs Outdated

pub struct Stdio;

impl serialport::SerialPort for Stdio {
Copy link
Author

@usbalbin usbalbin Oct 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A successful SerialPortBuilder::open returns Box<dyn SerialPort> however from what I have seen so far (might have missed something), we only seem to need io::Read and io::Write, if we could get a trait object of that instead of SerialPort for normal serial ports, we would not have to implement SerialPort for Stdio. Not sure if that is doable though...

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

implementing SerialPort for Stdio seems to be a bit hacky indeed. Could we create a new struct called Interface that implements io::Read and io::Write and then two builder functions: from_serial() and from_read() ?
Not sure how to handle the configuration of the serial port then (baud, parity etc)..

@hacknus
Copy link
Owner

hacknus commented Oct 24, 2023

Hi, first of all: thanks a lot for the contribution to this project!
Using the serial monitor like this is probably attractive only for a small amount of people but nevertheless it is still a cool feature and would be cool to implement this!
I'm currently busy with other stuff so I won't be able to contribute to this feature but I'm happy to check in from time to time.

Regarding this:

BTW, any ideas as to why the data does not show up until "number of plots" is increased to 2?

I'm aware of some bugs in the current release, specifically concerning the named lines in the plot and grouping etc. (see issue #80 )
This is going to be my next priority.

Comment on lines +1 to +31
use std::io;

pub enum Interface {
SerialPort(Box<dyn serialport::SerialPort>),
Stdio,
}

impl io::Write for Interface {
fn write(&mut self, buf: &[u8]) -> io::Result<usize> {
match self {
Interface::SerialPort(s) => s.write(buf),
Interface::Stdio => io::stdout().write(buf),
}
}

fn flush(&mut self) -> io::Result<()> {
match self {
Interface::SerialPort(s) => s.flush(),
Interface::Stdio => io::stdout().flush(),
}
}
}

impl io::Read for Interface {
fn read(&mut self, buf: &mut [u8]) -> io::Result<usize> {
match self {
Interface::SerialPort(s) => s.read(buf),
Interface::Stdio => io::stdin().read(buf),
}
}
}
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

implementing SerialPort for Stdio seems to be a bit hacky indeed. Could we create a new struct called Interface that implements io::Read and io::Write and then two builder functions: from_serial() and from_read() ?
Not sure how to handle the configuration of the serial port then (baud, parity etc)..

Something like this?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Serial settings are ignored. From ui point of view perhaps it would make most sense if the serial related things were greyed out once "stdio" is selected? However I am not sure how I would accomplish something like that..

Copy link
Owner

@hacknus hacknus Oct 27, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking more like this:

/// STDIO interface
#[derive(Debug)]
pub struct StdioInterface<stdio> {
    pub(crate) stdio: stdio,
}

/// Serial interface
#[derive(Debug)]
pub struct SerialInterface<SerialPort> {
    pub(crate) serial: SerialPort,
}


/// Write data
pub trait WriteData: private::Sealed {
    /// Error type
    type Error;
    /// Write data.
    fn write(&mut self, payload: &str) -> Result<(), Self::Error>;
   /// Flush data.
    fn flush(&mut self, payload: &str) -> Result<(), Self::Error>;
}

/// Read data
pub trait ReadData: private::Sealed {
    /// Error type
    type Error;
    /// Read some data.
    fn read(&mut self) -> Result<String, Self::Error>;
}

impl<SerialPort, E> ReadData for SerialInterface<SerialPort >
    where
        SerialPort: // traits
{
    type Error = Error<E>;
    // ... 
}

impl<stdio, E> ReadData for StdioInterface<stdio>
    where
        stdio: // traits
{
    type Error = Error<E>;
    // ... 
}

impl<SerialPort, E> WriteData for SerialInterface<SerialPort >
    where
        SerialPort: // traits
{
    type Error = Error<E>;
    // ... 
}

impl<stdio, E> WriteData for StdioInterface<stdio>
    where
        stdio: // traits
{
    type Error = Error<E>;
    // ... 
}

But not sure, if this works...
greying out the serial settings should be possible in the GUI.

Copy link
Author

@usbalbin usbalbin Nov 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unless I misunderstand, that would require casting Box<dyn SerialPort> into Box<dyn WriteData> which as far as I know is not possible. oh...

There is however also serialport::new_native which returns different things depending on OS. At that point we would no longer need the Box<dyn ...> and just #[cfg] into the right enum variant.

Copy link
Author

@usbalbin usbalbin Nov 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unless I misunderstand, that would require casting Box into Box which as far as I know is not possible. oh...

Oh I think maybe I understand now(?). Since the type is wrapped in a newtype it would be the newtype that could then be turned into a Box which would be fine. However that would be double boxing...

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, this is not as straight forward as I expected...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants