You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2022/02/08 21:25:37 UTC
[GitHub] [incubator-nuttx] patacongo opened a new pull request #5447: Added comment to wdog_start
patacongo opened a new pull request #5447:
URL: https://github.com/apache/incubator-nuttx/pull/5447
## Summary
Add a comment briefly explaining why one timer tick is added to the watchdog delay. This has confused a lot of people and has resulted in numerous awkward discussions and ill-informed attempts to modify the code to remove that addition. Perhaps a rather lengthy comment will reduce that confusion and put an end to this thrashing.
## Impact
None. Comment change only
## Testing
CI is sufficient
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] davids5 commented on a change in pull request #5447: Added comment to wdog_start
Posted by GitBox <gi...@apache.org>.
davids5 commented on a change in pull request #5447:
URL: https://github.com/apache/incubator-nuttx/pull/5447#discussion_r802113863
##########
File path: sched/wdog/wd_start.c
##########
@@ -204,7 +204,20 @@ int wd_start(FAR struct wdog_s *wdog, sclock_t delay,
up_getpicbase(&wdog->picbase);
wdog->arg = arg;
- /* Calculate delay+1, forcing the delay into a range that we can handle */
+ /* Calculate delay+1, forcing the delay into a range that we can handle.
+ *
+ * NOTE that one is added to the delay. This is correct and must not be
+ * changed: The contract for the use wdog_start is that the wdog will
+ * delay FOR AT LEAST as long as requested, but may delay longer due to
+ * variety of factors. The wdog logic has no knowledge of the the phase
+ * of the system timer when it is started: The next timer interrupt may
+ * occur immediately or may be delayed for almost a full cycle. In order
+ * to meet the contract requirement, the requested time is also always
+ * incremented by one so that the delay is always at least as long as
+ * requested.
+ *
+ * This is extensive documentation about this time issue elsewhere.
Review comment:
```suggestion
* There is extensive documentation about this time issue elsewhere.
```
It may be better to sight the reference or delete this sentence completely
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 merged pull request #5447: Added comment to wdog_start
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged pull request #5447:
URL: https://github.com/apache/incubator-nuttx/pull/5447
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on a change in pull request #5447: Added comment to wdog_start
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #5447:
URL: https://github.com/apache/incubator-nuttx/pull/5447#discussion_r803669170
##########
File path: sched/wdog/wd_start.c
##########
@@ -204,7 +204,20 @@ int wd_start(FAR struct wdog_s *wdog, sclock_t delay,
up_getpicbase(&wdog->picbase);
wdog->arg = arg;
- /* Calculate delay+1, forcing the delay into a range that we can handle */
+ /* Calculate delay+1, forcing the delay into a range that we can handle.
+ *
+ * NOTE that one is added to the delay. This is correct and must not be
+ * changed: The contract for the use wdog_start is that the wdog will
+ * delay FOR AT LEAST as long as requested, but may delay longer due to
+ * variety of factors. The wdog logic has no knowledge of the the phase
+ * of the system timer when it is started: The next timer interrupt may
+ * occur immediately or may be delayed for almost a full cycle. In order
+ * to meet the contract requirement, the requested time is also always
+ * incremented by one so that the delay is always at least as long as
+ * requested.
+ *
+ * This is extensive documentation about this time issue elsewhere.
Review comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org