Re: [PR] sched: Add max delay tick limitation for wdog/wqueue. [nuttx]
jerpelea merged PR #16394: URL: https://github.com/apache/nuttx/pull/16394 -- 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-unsubscr...@nuttx.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] sched: Add max delay tick limitation for wdog/wqueue. [nuttx]
xiaoxiang781216 commented on PR #16394: URL: https://github.com/apache/nuttx/pull/16394#issuecomment-2902060258 @Fix-Point why ci still fail? -- 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-unsubscr...@nuttx.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] sched: Add max delay tick limitation for wdog/wqueue. [nuttx]
jlaitine commented on PR #16394: URL: https://github.com/apache/nuttx/pull/16394#issuecomment-2900554383 Yes, I can confirm that it now works, I had a messed-up environment myself. -- 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-unsubscr...@nuttx.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] sched: Add max delay tick limitation for wdog/wqueue. [nuttx]
jlaitine commented on PR #16394: URL: https://github.com/apache/nuttx/pull/16394#issuecomment-293212 Hm. I am experiencing an assertion in the test on rv-virt:smp, with updated nuttx-apps and this PR still: [CPU1] Assertion failed : at file: wdog.c:312 task(CPU1): ostest process: ostest 0x80011b46 -- 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-unsubscr...@nuttx.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] sched: Add max delay tick limitation for wdog/wqueue. [nuttx]
Fix-Point commented on PR #16394: URL: https://github.com/apache/nuttx/pull/16394#issuecomment-2900063024 > Hm. I am experiencing an assertion in the test on rv-virt:smp, with updated nuttx-apps and this PR still: > > [CPU1] Assertion failed : at file: wdog.c:312 task(CPU1): ostest process: ostest 0x80011b46 > > Perhaps check what happens in the test when interval -1 is passed as an argument to wd_start... ? Rebased the PR and tested with the latest nuttx-apps(a5709532577ff0b3276a78bc8d278e8bf8a1b037). It works fine for me. -- 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-unsubscr...@nuttx.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] sched: Add max delay tick limitation for wdog/wqueue. [nuttx]
Fix-Point commented on code in PR #16394: URL: https://github.com/apache/nuttx/pull/16394#discussion_r2094984901 ## sched/wdog/wd_start.c: ## @@ -406,15 +406,12 @@ int wd_start_abstick(FAR struct wdog_s *wdog, clock_t ticks, * / -int wd_start(FAR struct wdog_s *wdog, sclock_t delay, +int wd_start(FAR struct wdog_s *wdog, clock_t delay, wdentry_t wdentry, wdparm_t arg) { - /* Verify the wdog and setup parameters */ + /* Ensure delay is within the range the wdog can handle. */ - if (delay < 0) -{ - return -EINVAL; -} + delay = delay < WDOG_MAX_DELAY ? delay : WDOG_MAX_DELAY; Review Comment: Done. I will modify the wdog test to adapt the semantic change. -- 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-unsubscr...@nuttx.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] sched: Add max delay tick limitation for wdog/wqueue. [nuttx]
jlaitine commented on code in PR #16394: URL: https://github.com/apache/nuttx/pull/16394#discussion_r2094903725 ## sched/wdog/wd_start.c: ## @@ -406,15 +406,12 @@ int wd_start_abstick(FAR struct wdog_s *wdog, clock_t ticks, * / -int wd_start(FAR struct wdog_s *wdog, sclock_t delay, +int wd_start(FAR struct wdog_s *wdog, clock_t delay, wdentry_t wdentry, wdparm_t arg) { - /* Verify the wdog and setup parameters */ + /* Ensure delay is within the range the wdog can handle. */ - if (delay < 0) -{ - return -EINVAL; -} + delay = delay < WDOG_MAX_DELAY ? delay : WDOG_MAX_DELAY; Review Comment: Would it be better to just return error if delay > WDOG_MAX_DELAY? If someone actually tries to set longer delays, just silently setting it to MAX will cause a hidden error for the user -- 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-unsubscr...@nuttx.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] sched: Add max delay tick limitation for wdog/wqueue. [nuttx]
jlaitine commented on code in PR #16394: URL: https://github.com/apache/nuttx/pull/16394#discussion_r2094903725 ## sched/wdog/wd_start.c: ## @@ -406,15 +406,12 @@ int wd_start_abstick(FAR struct wdog_s *wdog, clock_t ticks, * / -int wd_start(FAR struct wdog_s *wdog, sclock_t delay, +int wd_start(FAR struct wdog_s *wdog, clock_t delay, wdentry_t wdentry, wdparm_t arg) { - /* Verify the wdog and setup parameters */ + /* Ensure delay is within the range the wdog can handle. */ - if (delay < 0) -{ - return -EINVAL; -} + delay = delay < WDOG_MAX_DELAY ? delay : WDOG_MAX_DELAY; Review Comment: Would it be better to just return error if delay > WDOG_MAX_DELAY or delay < 0? If someone actually tries to set longer delays, just silently setting it to MAX will cause a hidden error for the user -- 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-unsubscr...@nuttx.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] sched: Add max delay tick limitation for wdog/wqueue. [nuttx]
Fix-Point commented on code in PR #16394: URL: https://github.com/apache/nuttx/pull/16394#discussion_r2094710251 ## Documentation/reference/os/time_clock.rst: ## @@ -494,7 +494,7 @@ with NuttX tasks. - :c:func:`wd_gettime` - Watchdog Timer Callback -.. c:function:: int wd_start(FAR struct wdog_s *wdog, int delay, \ +.. c:function:: int wd_start(FAR struct wdog_s *wdog, clock_t delay, \ Review Comment: The first commit is to address the problem that the workqueue delay parameter is not validated. -- 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-unsubscr...@nuttx.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org