<p><b>@kaspar030</b> commented on this pull request.</p>

<hr>

<p>In <a href="https://github.com/RIOT-OS/RIOT/pull/11527#discussion_r284627083">drivers/include/periph/wdt.h</a>:</p>
<pre style='color:#555'>> +void wdt_setup_reboot(uint32_t min_time, uint32_t max_time);
+
+#ifdef MODULE_PERIPH_WDT_CB
+/**
+ * @brief    Set up the wdt timer, only use max_time if normal operation
+ *           set min_time and max_time for windowed timer.
+ *
+ * @param[in] min_time       lower bound for windowed watchdog in ms, has to be 0
+ *                           for normal mode operation
+ * @param[in] max_time       upper bound for windowed watchdog in ms, time before
+ *                           reset for normal watchdog
+ * @param[in] wdt_cb         wdt callback, can be NULL
+ * @param[in] arg            optional argument which is passed to the
+ *                           callback, can be NULL
+ */
+void wdt_setup_callback(uint32_t min_time, uint32_t max_time, wdt_cb_t wdt_cb, void* arg);
</pre>
<blockquote>
<p>I think this can be misleading: The watchdog will still trigger a reboot and behave just like with wdt_setup_reboot, the only difference being that wdt_cb is executed before the watchdog timer expires.</p>
</blockquote>
<p>Maybe call it <code>wdt_setup_reset_with_warning()</code>?</p>
<p>BTW, we need an additional define for the time difference between warning callback and actual reset.</p>
<blockquote>
<p>Having two functions for it make it seem like <code>wdt_setup_callback</code> would cause the callback to be called <em>instead</em> of the reboot, not in addition to it.</p>
</blockquote>
<p>If this is one function, the functionality cannot be guarded properly. When supported, all callback handling code must be compiled in, even if unused.<br>
In practice, <code>wdt_setup_reset_with_warning()</code> will probably look like</p>
<pre><code>wdt_setup_reset_with_warning() {
 wdt_setup_reset();
// set callback
}
</code></pre>
<blockquote>
<p>Also it would be good if the function can return an error code if an invalid configuration is requested.</p>
</blockquote>
<p>Would be good for what? Can we get away with asserts and log messages?</p>
<pre><code>if (wdt_setup_reset() == EINVAL) {
  // now what? set a flag disabling wdt_kick()?
  // let the user know?
}
</code></pre>

<p style="font-size:small;-webkit-text-size-adjust:none;color:#666;">—<br />You are receiving this because you are subscribed to this thread.<br />Reply to this email directly, <a href="https://github.com/RIOT-OS/RIOT/pull/11527?email_source=notifications&email_token=ABE7WYGUZGILFISYKFKUW3TPVUU55A5CNFSM4HNCUZR2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOBY2AA7A#discussion_r284627083">view it on GitHub</a>, or <a href="https://github.com/notifications/unsubscribe-auth/ABE7WYFNRSSIDNXPNSNR74LPVUU55ANCNFSM4HNCUZRQ">mute the thread</a>.<img src="https://github.com/notifications/beacon/ABE7WYG7DC73HZLUFGAT7NLPVUU55A5CNFSM4HNCUZR2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOBY2AA7A.gif" height="1" width="1" alt="" /></p>
<script type="application/ld+json">[
{
"@context": "http://schema.org",
"@type": "EmailMessage",
"potentialAction": {
"@type": "ViewAction",
"target": "https://github.com/RIOT-OS/RIOT/pull/11527?email_source=notifications\u0026email_token=ABE7WYGUZGILFISYKFKUW3TPVUU55A5CNFSM4HNCUZR2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOBY2AA7A#discussion_r284627083",
"url": "https://github.com/RIOT-OS/RIOT/pull/11527?email_source=notifications\u0026email_token=ABE7WYGUZGILFISYKFKUW3TPVUU55A5CNFSM4HNCUZR2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOBY2AA7A#discussion_r284627083",
"name": "View Pull Request"
},
"description": "View this Pull Request on GitHub",
"publisher": {
"@type": "Organization",
"name": "GitHub",
"url": "https://github.com"
}
}
]</script>