Remove nix dependency #109
Conversation
Codecov Report
@@ Coverage Diff @@
## main #109 +/- ##
==========================================
+ Coverage 56.91% 60.77% +3.85%
==========================================
Files 56 59 +3
Lines 5508 5889 +381
==========================================
+ Hits 3135 3579 +444
+ Misses 2373 2310 -63
|
| libc::SOF_TIMESTAMPING_RAW_HARDWARE | ||
| | libc::SOF_TIMESTAMPING_RX_HARDWARE | ||
| | libc::SOF_TIMESTAMPING_TX_HARDWARE | ||
| | libc::SOF_TIMESTAMPING_OPT_TSONLY | ||
| | libc::SOF_TIMESTAMPING_OPT_ID |
There was a problem hiding this comment.
when did we last test with hardware timestamping? were the hardware timestamps actually used, because it can be hard to tell. Based on my testing for ntpd-rs, on our desktop machines at least, we need software timestamps to also be enabled for anything to happen.
There was a problem hiding this comment.
This does work on both the intel i210 and x710 cards.
|
|
||
| fn recv_with_timestamp( | ||
| tc_socket: &std::net::UdpSocket, | ||
| clock: &LinuxClock, |
There was a problem hiding this comment.
passing in the clock here is extremely inelegant. On the other hand, maybe this generates (much) more accurate timestamps and that is important? can we determine this timestamp outside of the UDP logic, or would that be too inaccurate?
There was a problem hiding this comment.
We can, but that leads to a more complicated interface that users of the library need to implement. The code always needs timestamps on the time critical socket, so the network abstraction should just provide them.
There was a problem hiding this comment.
Most of the nix replacements look good, but I don't like the changes to the interface made. When sending a message on the time critical socket, we always need a timestamp back, so any fallback should (in my view) just be generated inside the send function.
Furthermore, this needs some rebasing to use the new (correct) epoll mechanism.
folkertdev
force-pushed
the
remove-nix-dependency
branch
2 times, most recently
from
cb68b19
to
9792532
Compare
| libc::SOF_TIMESTAMPING_RAW_HARDWARE | ||
| | libc::SOF_TIMESTAMPING_RX_HARDWARE | ||
| | libc::SOF_TIMESTAMPING_TX_HARDWARE | ||
| | libc::SOF_TIMESTAMPING_OPT_TSONLY | ||
| | libc::SOF_TIMESTAMPING_OPT_ID |
folkertdev
force-pushed
the
remove-nix-dependency
branch
from
9792532
to
3dc9df9
Compare
this is a big one. The
nixdependency is gone, and a lot of logic has been imported from ntpd-rs. As far as I can establish, this PR does not regress any functionality, but 1) this is hard to actually verify and 2) it does expose some problems with the existing/prior implementation. I'll highlight these below.