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/09/18 13:15:51 UTC
[GitHub] [incubator-nuttx] xiaoxiang781216 opened a new pull request, #7126: watchdog: Support auto monitor keep alive from timer、 lowerhalf
xiaoxiang781216 opened a new pull request, #7126:
URL: https://github.com/apache/incubator-nuttx/pull/7126
## Summary
- watchdog: Rename WATCHDOG_AUTOMONITOR_BY_TIMER to WATCHDOG_AUTOMONITOR_BY_WDOG
- watchdog: Support auto monitor keep alive from timer lowerhalf
- watchdog: Support auto monitor keep alive from oneshot lowerhalf
## Impact
New funcionality
## Testing
Pass CI
--
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] pkarashchenko commented on a diff in pull request #7126: watchdog: Support auto monitor keep alive from timer/oneshot lowerhalf
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #7126:
URL: https://github.com/apache/incubator-nuttx/pull/7126#discussion_r975608048
##########
drivers/timers/watchdog.c:
##########
@@ -144,17 +148,50 @@ static int watchdog_automonitor_capture(int irq, FAR void *context,
return 0;
}
+#elif defined(CONFIG_WATCHDOG_AUTOMONITOR_BY_ONESHOT)
+static void
+watchdog_automonitor_oneshot(FAR struct oneshot_lowerhalf_s *oneshot,
+ FAR void *arg)
+{
+ FAR struct watchdog_upperhalf_s *upper = arg;
+ FAR struct watchdog_lowerhalf_s *lower = upper->lower;
+
+ if (upper->monitor)
+ {
+ struct timespec ts =
+ {
+ WATCHDOG_AUTOMONITOR_PING_INTERVAL, 0
+ };
Review Comment:
np. I observed that some places indent in different way. I'm fine with current
--
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 commented on a diff in pull request #7126: watchdog: Support auto monitor keep alive from timer/oneshot lowerhalf
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #7126:
URL: https://github.com/apache/incubator-nuttx/pull/7126#discussion_r975403684
##########
drivers/timers/watchdog.c:
##########
@@ -186,26 +223,49 @@ static void watchdog_automonitor_idle(FAR struct pm_callback_s *cb,
#endif
#ifdef CONFIG_WATCHDOG_AUTOMONITOR
-static void watchdog_automonitor_start(FAR struct watchdog_upperhalf_s
- *upper)
+# if defined(CONFIG_WATCHDOG_AUTOMONITOR_BY_ONESHOT)
+static void
+watchdog_automonitor_start(FAR struct watchdog_upperhalf_s *upper,
+ FAR struct oneshot_lowerhalf_s *oneshot)
+# elif defined(CONFIG_WATCHDOG_AUTOMONITOR_BY_TIMER)
+static void
+watchdog_automonitor_start(FAR struct watchdog_upperhalf_s *upper,
+ FAR struct timer_lowerhalf_s *timer)
+# else
+static void
+watchdog_automonitor_start(FAR struct watchdog_upperhalf_s *upper)
+# endif
{
FAR struct watchdog_lowerhalf_s *lower = upper->lower;
if (!upper->monitor)
{
- upper->monitor = true;
-#if defined(CONFIG_WATCHDOG_AUTOMONITOR_BY_CAPTURE)
+# if defined(CONFIG_WATCHDOG_AUTOMONITOR_BY_CAPTURE)
lower->ops->capture(lower, watchdog_automonitor_capture);
+# elif defined(CONFIG_WATCHDOG_AUTOMONITOR_BY_ONESHOT)
+ struct timespec ts =
+ {
+ WATCHDOG_AUTOMONITOR_PING_INTERVAL, 0
+ };
Review Comment:
Reply before.
--
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 commented on pull request #7126: watchdog: Support auto monitor keep alive from timer/oneshot lowerhalf
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on PR #7126:
URL: https://github.com/apache/incubator-nuttx/pull/7126#issuecomment-1252698017
> Seems to be a "breaking change" due to default value change for `Auto-monitor keepalive by`
But, the final result is same: the ping interval is still half of timeout.
--
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] pkarashchenko commented on pull request #7126: watchdog: Support auto monitor keep alive from timer/oneshot lowerhalf
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on PR #7126:
URL: https://github.com/apache/incubator-nuttx/pull/7126#issuecomment-1252205649
Seems to be a "breaking change" due to default value change for `Auto-monitor keepalive by`
--
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 commented on a diff in pull request #7126: watchdog: Support auto monitor keep alive from timer/oneshot lowerhalf
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #7126:
URL: https://github.com/apache/incubator-nuttx/pull/7126#discussion_r974162558
##########
drivers/timers/Kconfig:
##########
@@ -366,13 +366,21 @@ if WATCHDOG_AUTOMONITOR
choice
prompt "Auto-monitor keepalive by"
- default WATCHDOG_AUTOMONITOR_BY_TIMER
+ default WATCHDOG_AUTOMONITOR_BY_WDOG
config WATCHDOG_AUTOMONITOR_BY_CAPTURE
bool "Capture callback"
Review Comment:
Done.
##########
drivers/timers/Kconfig:
##########
@@ -366,13 +366,21 @@ if WATCHDOG_AUTOMONITOR
choice
prompt "Auto-monitor keepalive by"
- default WATCHDOG_AUTOMONITOR_BY_TIMER
+ default WATCHDOG_AUTOMONITOR_BY_WDOG
config WATCHDOG_AUTOMONITOR_BY_CAPTURE
bool "Capture callback"
+config WATCHDOG_AUTOMONITOR_BY_ONESHOT
+ bool "Oneshot callback"
+ depends on ONESHOT
+
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
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #7126: watchdog: Support auto monitor keep alive from timer/oneshot lowerhalf
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #7126:
URL: https://github.com/apache/incubator-nuttx/pull/7126#discussion_r975403344
##########
drivers/timers/watchdog.c:
##########
@@ -144,17 +148,50 @@ static int watchdog_automonitor_capture(int irq, FAR void *context,
return 0;
}
+#elif defined(CONFIG_WATCHDOG_AUTOMONITOR_BY_ONESHOT)
+static void
+watchdog_automonitor_oneshot(FAR struct oneshot_lowerhalf_s *oneshot,
+ FAR void *arg)
+{
+ FAR struct watchdog_upperhalf_s *upper = arg;
+ FAR struct watchdog_lowerhalf_s *lower = upper->lower;
+
+ if (upper->monitor)
+ {
+ struct timespec ts =
+ {
+ WATCHDOG_AUTOMONITOR_PING_INTERVAL, 0
+ };
Review Comment:
But most place doesn't indent in this case.
--
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] pkarashchenko commented on a diff in pull request #7126: watchdog: Support auto monitor keep alive from timer/oneshot lowerhalf
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #7126:
URL: https://github.com/apache/incubator-nuttx/pull/7126#discussion_r975608048
##########
drivers/timers/watchdog.c:
##########
@@ -144,17 +148,50 @@ static int watchdog_automonitor_capture(int irq, FAR void *context,
return 0;
}
+#elif defined(CONFIG_WATCHDOG_AUTOMONITOR_BY_ONESHOT)
+static void
+watchdog_automonitor_oneshot(FAR struct oneshot_lowerhalf_s *oneshot,
+ FAR void *arg)
+{
+ FAR struct watchdog_upperhalf_s *upper = arg;
+ FAR struct watchdog_lowerhalf_s *lower = upper->lower;
+
+ if (upper->monitor)
+ {
+ struct timespec ts =
+ {
+ WATCHDOG_AUTOMONITOR_PING_INTERVAL, 0
+ };
Review Comment:
np. I observer that some places indent in different way. I'm fine with current
--
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] pkarashchenko commented on a diff in pull request #7126: watchdog: Support auto monitor keep alive from timer/oneshot lowerhalf
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #7126:
URL: https://github.com/apache/incubator-nuttx/pull/7126#discussion_r975221624
##########
drivers/timers/watchdog.c:
##########
@@ -186,26 +223,49 @@ static void watchdog_automonitor_idle(FAR struct pm_callback_s *cb,
#endif
#ifdef CONFIG_WATCHDOG_AUTOMONITOR
-static void watchdog_automonitor_start(FAR struct watchdog_upperhalf_s
- *upper)
+# if defined(CONFIG_WATCHDOG_AUTOMONITOR_BY_ONESHOT)
+static void
+watchdog_automonitor_start(FAR struct watchdog_upperhalf_s *upper,
+ FAR struct oneshot_lowerhalf_s *oneshot)
+# elif defined(CONFIG_WATCHDOG_AUTOMONITOR_BY_TIMER)
+static void
+watchdog_automonitor_start(FAR struct watchdog_upperhalf_s *upper,
+ FAR struct timer_lowerhalf_s *timer)
+# else
+static void
+watchdog_automonitor_start(FAR struct watchdog_upperhalf_s *upper)
+# endif
{
FAR struct watchdog_lowerhalf_s *lower = upper->lower;
if (!upper->monitor)
{
- upper->monitor = true;
-#if defined(CONFIG_WATCHDOG_AUTOMONITOR_BY_CAPTURE)
+# if defined(CONFIG_WATCHDOG_AUTOMONITOR_BY_CAPTURE)
lower->ops->capture(lower, watchdog_automonitor_capture);
+# elif defined(CONFIG_WATCHDOG_AUTOMONITOR_BY_ONESHOT)
+ struct timespec ts =
+ {
+ WATCHDOG_AUTOMONITOR_PING_INTERVAL, 0
+ };
Review Comment:
```suggestion
{
WATCHDOG_AUTOMONITOR_PING_INTERVAL, 0
};
```
##########
drivers/timers/watchdog.c:
##########
@@ -144,17 +148,50 @@ static int watchdog_automonitor_capture(int irq, FAR void *context,
return 0;
}
+#elif defined(CONFIG_WATCHDOG_AUTOMONITOR_BY_ONESHOT)
+static void
+watchdog_automonitor_oneshot(FAR struct oneshot_lowerhalf_s *oneshot,
+ FAR void *arg)
+{
+ FAR struct watchdog_upperhalf_s *upper = arg;
+ FAR struct watchdog_lowerhalf_s *lower = upper->lower;
+
+ if (upper->monitor)
+ {
+ struct timespec ts =
+ {
+ WATCHDOG_AUTOMONITOR_PING_INTERVAL, 0
+ };
Review Comment:
```suggestion
{
WATCHDOG_AUTOMONITOR_PING_INTERVAL, 0
};
```
I see that style accept both, so not sure which one is better
--
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] acassis commented on a diff in pull request #7126: watchdog: Support auto monitor keep alive from timer/oneshot lowerhalf
Posted by GitBox <gi...@apache.org>.
acassis commented on code in PR #7126:
URL: https://github.com/apache/incubator-nuttx/pull/7126#discussion_r974109681
##########
drivers/timers/Kconfig:
##########
@@ -366,13 +366,21 @@ if WATCHDOG_AUTOMONITOR
choice
prompt "Auto-monitor keepalive by"
- default WATCHDOG_AUTOMONITOR_BY_TIMER
+ default WATCHDOG_AUTOMONITOR_BY_WDOG
config WATCHDOG_AUTOMONITOR_BY_CAPTURE
bool "Capture callback"
+config WATCHDOG_AUTOMONITOR_BY_ONESHOT
+ bool "Oneshot callback"
+ depends on ONESHOT
+
Review Comment:
Please include a Help description about this symbol, when know what an Oneshot callback is, but a newcomer user could have no clue.
##########
drivers/timers/Kconfig:
##########
@@ -366,13 +366,21 @@ if WATCHDOG_AUTOMONITOR
choice
prompt "Auto-monitor keepalive by"
- default WATCHDOG_AUTOMONITOR_BY_TIMER
+ default WATCHDOG_AUTOMONITOR_BY_WDOG
config WATCHDOG_AUTOMONITOR_BY_CAPTURE
bool "Capture callback"
Review Comment:
Please include a Help description about this symbol, a newcomer user could become confused about it.
##########
drivers/timers/Kconfig:
##########
@@ -366,13 +366,21 @@ if WATCHDOG_AUTOMONITOR
choice
prompt "Auto-monitor keepalive by"
- default WATCHDOG_AUTOMONITOR_BY_TIMER
+ default WATCHDOG_AUTOMONITOR_BY_WDOG
config WATCHDOG_AUTOMONITOR_BY_CAPTURE
bool "Capture callback"
+config WATCHDOG_AUTOMONITOR_BY_ONESHOT
+ bool "Oneshot callback"
+ depends on ONESHOT
+
config WATCHDOG_AUTOMONITOR_BY_TIMER
bool "Timer callback"
+ depends on TIMER
+
+config WATCHDOG_AUTOMONITOR_BY_WDOG
+ bool "Wdog callback"
config WATCHDOG_AUTOMONITOR_BY_WORKER
bool "Worker callback"
Review Comment:
And also to Idle callback, we need to improve our documentation and have more info on Kconfig makes user's life better
##########
drivers/timers/Kconfig:
##########
@@ -366,13 +366,21 @@ if WATCHDOG_AUTOMONITOR
choice
prompt "Auto-monitor keepalive by"
- default WATCHDOG_AUTOMONITOR_BY_TIMER
+ default WATCHDOG_AUTOMONITOR_BY_WDOG
config WATCHDOG_AUTOMONITOR_BY_CAPTURE
bool "Capture callback"
+config WATCHDOG_AUTOMONITOR_BY_ONESHOT
+ bool "Oneshot callback"
+ depends on ONESHOT
+
config WATCHDOG_AUTOMONITOR_BY_TIMER
bool "Timer callback"
+ depends on TIMER
Review Comment:
Ditto, also it could be useful to give some hints what/how he will configue the timer later
##########
drivers/timers/Kconfig:
##########
@@ -366,13 +366,21 @@ if WATCHDOG_AUTOMONITOR
choice
prompt "Auto-monitor keepalive by"
- default WATCHDOG_AUTOMONITOR_BY_TIMER
+ default WATCHDOG_AUTOMONITOR_BY_WDOG
config WATCHDOG_AUTOMONITOR_BY_CAPTURE
bool "Capture callback"
+config WATCHDOG_AUTOMONITOR_BY_ONESHOT
+ bool "Oneshot callback"
+ depends on ONESHOT
+
config WATCHDOG_AUTOMONITOR_BY_TIMER
bool "Timer callback"
+ depends on TIMER
+
+config WATCHDOG_AUTOMONITOR_BY_WDOG
+ bool "Wdog callback"
Review Comment:
Ditto
##########
drivers/timers/Kconfig:
##########
@@ -366,13 +366,21 @@ if WATCHDOG_AUTOMONITOR
choice
prompt "Auto-monitor keepalive by"
- default WATCHDOG_AUTOMONITOR_BY_TIMER
+ default WATCHDOG_AUTOMONITOR_BY_WDOG
config WATCHDOG_AUTOMONITOR_BY_CAPTURE
bool "Capture callback"
+config WATCHDOG_AUTOMONITOR_BY_ONESHOT
+ bool "Oneshot callback"
+ depends on ONESHOT
+
config WATCHDOG_AUTOMONITOR_BY_TIMER
bool "Timer callback"
+ depends on TIMER
+
+config WATCHDOG_AUTOMONITOR_BY_WDOG
+ bool "Wdog callback"
config WATCHDOG_AUTOMONITOR_BY_WORKER
bool "Worker callback"
Review Comment:
Ditto
--
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 commented on a diff in pull request #7126: watchdog: Support auto monitor keep alive from timer/oneshot lowerhalf
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #7126:
URL: https://github.com/apache/incubator-nuttx/pull/7126#discussion_r974162758
##########
drivers/timers/Kconfig:
##########
@@ -366,13 +366,21 @@ if WATCHDOG_AUTOMONITOR
choice
prompt "Auto-monitor keepalive by"
- default WATCHDOG_AUTOMONITOR_BY_TIMER
+ default WATCHDOG_AUTOMONITOR_BY_WDOG
config WATCHDOG_AUTOMONITOR_BY_CAPTURE
bool "Capture callback"
+config WATCHDOG_AUTOMONITOR_BY_ONESHOT
+ bool "Oneshot callback"
+ depends on ONESHOT
+
config WATCHDOG_AUTOMONITOR_BY_TIMER
bool "Timer callback"
+ depends on TIMER
+
+config WATCHDOG_AUTOMONITOR_BY_WDOG
+ bool "Wdog callback"
Review Comment:
Done
##########
drivers/timers/Kconfig:
##########
@@ -366,13 +366,21 @@ if WATCHDOG_AUTOMONITOR
choice
prompt "Auto-monitor keepalive by"
- default WATCHDOG_AUTOMONITOR_BY_TIMER
+ default WATCHDOG_AUTOMONITOR_BY_WDOG
config WATCHDOG_AUTOMONITOR_BY_CAPTURE
bool "Capture callback"
+config WATCHDOG_AUTOMONITOR_BY_ONESHOT
+ bool "Oneshot callback"
+ depends on ONESHOT
+
config WATCHDOG_AUTOMONITOR_BY_TIMER
bool "Timer callback"
+ depends on TIMER
+
+config WATCHDOG_AUTOMONITOR_BY_WDOG
+ bool "Wdog callback"
config WATCHDOG_AUTOMONITOR_BY_WORKER
bool "Worker callback"
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
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #7126: watchdog: Support auto monitor keep alive from timer/oneshot lowerhalf
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #7126:
URL: https://github.com/apache/incubator-nuttx/pull/7126#discussion_r974162980
##########
drivers/timers/Kconfig:
##########
@@ -366,13 +366,21 @@ if WATCHDOG_AUTOMONITOR
choice
prompt "Auto-monitor keepalive by"
- default WATCHDOG_AUTOMONITOR_BY_TIMER
+ default WATCHDOG_AUTOMONITOR_BY_WDOG
config WATCHDOG_AUTOMONITOR_BY_CAPTURE
bool "Capture callback"
+config WATCHDOG_AUTOMONITOR_BY_ONESHOT
+ bool "Oneshot callback"
+ depends on ONESHOT
+
config WATCHDOG_AUTOMONITOR_BY_TIMER
bool "Timer callback"
+ depends on TIMER
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
[GitHub] [incubator-nuttx] pkarashchenko merged pull request #7126: watchdog: Support auto monitor keep alive from timer/oneshot lowerhalf
Posted by GitBox <gi...@apache.org>.
pkarashchenko merged PR #7126:
URL: https://github.com/apache/incubator-nuttx/pull/7126
--
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