nal: Fix loops & socket handle management #7

Merged
harry merged 1 commits from fix-nal into master 2024-08-17 17:37:23 +08:00
1 changed files with 3 additions and 1 deletions
Showing only changes of commit 232a08f110 - Show all commits

View File

@ -194,7 +194,7 @@ where
}
}
};
// Blocking connect
// Loop to wait until the socket is staying established or closed,
// or the connection attempt has timed out.
let mut timeout_ms: u32 = 0;
@ -216,11 +216,13 @@ where
return Ok(handle)
}
}
// Any TCP states other than CLOSED and ESTABLISHED are considered
// "transient", so this function should keep waiting and let smoltcp poll
// (e.g. for handling echo reqeust/reply) at the same time.
timeout_ms += self.update()?;
Review

While all the other steps are very nicely documented, this fairly important self.poll() disappears to my eyes. How about a pair of newlines around it?

While all the other steps are very nicely documented, this fairly important `self.poll()` disappears to my eyes. How about a pair of newlines around it?
Review

Right. Before, poll() was simply part of update(), and I decided to separate them simply for the sake of clarity.

What do you think: is it good to keep them separated?

Right. Before, `poll()` was simply part of `update()`, and I decided to separate them simply for the sake of clarity. What do you think: is it good to keep them separated?
Review

Smaller chunks with separated logic are always better.

Smaller chunks with separated logic are always better.
Review

In 232a08f110, I added a pair of newlines, one before the comment line (that explains polling), and one after self.poll(). Would that be good?

In 232a08f11012fd749b68882140a8b9dc1a653363, I added a pair of newlines, one before the comment line (that explains polling), and one after `self.poll()`. Would that be good?
self.poll()?;
// Time out, and return the socket for re-connection in the future.
if timeout_ms > self.connection_timeout_ms {
// TODO: Return Err(), but would require changes in quartiq/minimq