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

Received spurious ACK #40

Open
milo1000 opened this issue Dec 3, 2022 · 4 comments
Open

Received spurious ACK #40

milo1000 opened this issue Dec 3, 2022 · 4 comments
Labels

Comments

@milo1000
Copy link
Contributor

milo1000 commented Dec 3, 2022

Scenario for this problem:

  1. Host (using min.py) sends some frames to client (which is Arduino with min.c)
  2. Arduino listens for incoming frames and sends "keep alive" frames to notify host that everything is OK
  3. At that point Arduino client is restarted (hard-reset)
  4. Host keeps sending ACK frames for some old seq, client prints "Received spurious ACK" and basically stops working
  5. Due to lack of "keep alive" frames, Host decides to perform transport_reset() - that resets client, but unfortunately Host keeps sending "spurious ACK"
    To fix that I've added self._rn = 0 to _transport_fifo_reset, it seems to work OK, but need to be confirmed by someone with expertise.
def _transport_fifo_reset(self):
        self._transport_fifo = []
        self._last_received_anything_ms = self._now_ms()
        self._last_sent_ack_time_ms = self._now_ms()
        self._last_sent_frame_ms = 0
        self._last_received_frame_ms = 0
        self._sn_min = 0
        self._sn_max = 0
        self._rn = 0
@kentindell
Copy link
Collaborator

Yes, that should include that (it mirrors the embedded side). Can you make a Pull Request? I can then accept it.

@kentindell kentindell added the bug label Dec 29, 2022
@subodh-malgonde
Copy link

Looks like this issue has been fixed #42

0x3333 added a commit to 0x3333/min that referenced this issue Oct 27, 2024
@schperplata
Copy link

I am still getting the exact same Spurious ACK issue. @milo1000, did the fix #42 resolved your issue?

@milo1000
Copy link
Contributor Author

Yes, it did.

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

No branches or pull requests

4 participants