[riot-notifications] [RIOT-OS/RIOT] gcoap: don't call random_uint32_range() when COAP_ACK_VARIANCE=0 (#11784)
notifications at github.com
Wed Jul 31 11:35:03 CEST 2019
> I understand the need for conditional compilation here, and the code tests fine. However, there is a larger issue here related to the first question in the Maintainer Guidelines. This question also applies to #11787, so we might as well discuss it here for both PRs.
> > Does the reasoning for this PR make sense?
> To me, "make sense" includes whether a user in a non-experimental setup would need this PR. It seems like if we open up the mainline code to what is needed for experiments, then that significantly increases the scope of what is acceptable. If it's just needed for an experiment, then why not just create a branch with the modifications for that purpose. Do we have a policy here?
I can just argue from the past and we do have quite a lot of features providing functionality for experimentation (which also come into use for testing sometimes). Just to mention a few: `netstat_t`, `gnrc_pktbuf_stats()`, or even `gnrc_netreg_register()` (if you register additional subscribers). I also think testing can benefit from this setup. During our experiments we noticed that when taking out the back-off and the randomization, that there is a slight drift in the retransmissions (I was not able to reproduce it isolated, that's why I did not report it yet), but for finding these kind of bugs, reducing the retransmission behavior to a minimum of what's necessary can also be beneficial.
> At any rate, I think this PR is marginally acceptable for my "non-experimental" test. OTOH, it's hard to imagine how #11787 would be useful in a non-experimental setup.
> I don't feel super strongly about this, but I hate to reduce readability in mainline code with conditionals that only a single experimental setup will ever need.
(1) I don't really think readability is decreased to such a degree, that it raises concerns, (2) it is not a single experimental setup. Everytime reliability and timing needs to be compared to protocols that do not have randomizations or backoffs in their retransmission mechanism, this will come up. Putting this into its own branch, could be an option, but then the RIOT version get's a little bit more complicated to reference in a given paper (weak argument, I know ;-)).
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the notifications