Add a label to timeouts for better errors #320
Conversation
fpdotmonkey
force-pushed
the
labeled-timeouts
branch
2 times, most recently
from
00c050e to
0fab93e
Compare
|
netbsd failed on |
jamwaffles
left a comment
jamwaffles
left a comment
There was a problem hiding this comment.
A few nits and reorg bits, otherwise this is really good, thank you! I too get those dreaded Init: Timeout errors with no recourse but to drag Wireshark out. It will be nice to have a middle ground.
|
Don't worry about the netbsd tests. Not sure why they're failing but everything else is fine. Also I just noticed |
I think |
fpdotmonkey
force-pushed
the
labeled-timeouts
branch
2 times, most recently
from
732e2fd to
00959df
Compare
Up to this commit, timeout errors have been inspecific about which timeout is being tripped. This can be frustrating, as timeout errors are user-facing and often require action by the user, either to adjust timeout durations or to do deeper diagnosis on the hardware. This creates better error messages by adding a label to all timeouts so that they will report which particular timeout they tripped. The other most notable change is that `crate::timer_factory::TimeoutFuture` has had it's `.duration` field added regardless of whether it's built with miri. I needed to add `TimeoutKind` to this struct no matter what and I felt it would be silly to separate it from `Duration` just for miri's sake. I've tested this on Beckhoff's EK1100, EL3104, EL4134, EL6224, EL1259, EL6751, HBM's TIM EC, and Bosch-Rexroth's VT-HMC. The latter also exercised the PDU timeouts in addition to the regular timer factory ones.
(and some minor cleanup in that file)
jamwaffles
force-pushed
the
labeled-timeouts
branch
from
00959df to
91337bb
Compare
jamwaffles
left a comment
jamwaffles
left a comment
There was a problem hiding this comment.
Sorry for the massive delay on this. I've pushed a changelog entry commit, now verything looks good, thanks again!
2 participants
Up to this commit, timeout errors have been inspecific about which timeout is being tripped. This can be frustrating, as timeout errors are user-facing and often require action by the user, either to adjust timeout durations or to do deeper diagnosis on the hardware.
This creates better error messages by adding a label to all timeouts so that they will report which particular timeout they tripped.
The other most notable change is that
crate::timer_factory::TimeoutFuturehas had it's.durationfield added regardless of whether it's built with miri. I needed to addTimeoutKindto this struct no matter what and I felt it would be silly to separate it fromDurationjust for miri's sake.I've tested this on Beckhoff's EK1100, EL3104, EL4134, EL6224, EL1259, EL6751, HBM's TIM EC, and Bosch-Rexroth's VT-HMC. The latter also exercised the PDU timeouts in addition to the regular timer factory ones.