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

feat(init-reset): implemented small state machine for link opening and reset #9

Merged
merged 6 commits into from
Dec 10, 2024

Conversation

gabrik
Copy link
Collaborator

@gabrik gabrik commented Dec 4, 2024

Initializaiton message exchange from cold start

  1. Connect side sends a message with empty payload and I flag set.
  2. Listen side sends a message with empty payload and I+A flags set.
  3. Data can be exchanged from this point on.
///  ┌─────────┐                ┌──────────┐   
///  │ Listen  │                │ Connect  │   
///  └────┬────┘                └────┬─────┘   
///       │                          │         
///       │       Init               │         
///       │◄─────────────────────────┤         
///       │                          │         
///       │        Init + Ack        │         
///       ├─────────────────────────►│         
///       │       Data               │         
///       │◄─────────────────────────┤         
///       ├─────────────────────────►│         
///       │                          │    

If connect sides restarts:

  1. Connect side sends a message with empty payload and I flag set.
  2. Listen side sends a message with empty payload and R flag set.
  3. Connect side waits and goes back to cold start case.
///   ┌─────────┐                ┌──────────┐
///   │ Listen  │                │ Connect  │
///   └────┬────┘                └────┬─────┘
///        │        Init              │       
///        │◄─────────────────────────┤       
///        │       Reset              │       
///        ├─────────────────────────►│       
///        │                          │       
///        │                          │       
///        │                          │       
///        │        Init              │       
///        │◄─────────────────────────┤       
///        │       Init + Ack         │       
///        ├─────────────────────────►│       
///        │         Data             │       
///        │◄─────────────────────────┤       
///        ├─────────────────────────►│       
///        │                          │       
///        │                          │  

…d reset

Signed-off-by: Gabriele Baldoni <gabriele.baldoni@gmail.com>
src/lib.rs Outdated
break;
} else if hdr.has_r_flag() {
// we received a reset we must resend the init message after a small sleep
tokio::time::sleep(Duration::from_micros(SERIAL_CONNECT_THROTTLE_TIME_US)).await;
Copy link

Choose a reason for hiding this comment

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

Is this sleep really necessary?

Signed-off-by: Gabriele Baldoni <gabriele.baldoni@gmail.com>
@ur8us
Copy link

ur8us commented Dec 4, 2024

Guys, I am sorry for the headache, but it is clearly seen from the diagram that the behavior of the Connect side does not depend on the state of the flags in the new header field (the "H" byte). If the connect request is successful, data transfer begins. If not, the Connect side keeps trying. In other words, the H byte is useless. :-(

src/lib.rs Outdated
log::trace!("Received header: {hdr:02X?}");
if hdr.has_i_flag() {
// we send back a message with both I and A flags
self.internal_write(&[0u8], Header::new(I_FLAG | A_FLAG))
Copy link
Contributor

Choose a reason for hiding this comment

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

It is not an empty payload, but payload with length 1, but according to the format we expected length 0

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Indeed, but we do not even check the payload. Its indeed a payload with lenght 1 and a single 0 byte.

Copy link
Contributor

Choose a reason for hiding this comment

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

We don't check the payload here, but in pico the message is read in its entirety, and for this a buffer is needed, for connection messages I would prefer not to allocate it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm going to fix it.

Signed-off-by: Gabriele Baldoni <gabriele.baldoni@gmail.com>
Signed-off-by: Gabriele Baldoni <gabriele.baldoni@gmail.com>
Signed-off-by: Gabriele Baldoni <gabriele.baldoni@gmail.com>
Signed-off-by: Gabriele Baldoni <gabriele.baldoni@gmail.com>
@gabrik gabrik merged commit 1ccb5f0 into main Dec 10, 2024
@gabrik gabrik deleted the feat/state-machine branch December 10, 2024 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants