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 2021/05/26 18:46:00 UTC
[GitHub] [incubator-nuttx] Ouss4 opened a new pull request #3789: esp32&esp32-c3/rt_timer: Re-word some comments and fix typos
Ouss4 opened a new pull request #3789:
URL: https://github.com/apache/incubator-nuttx/pull/3789
## Summary
Re-word some comments and fix typos in `rt_timer` files in both ESP32 and ESP32-C3.
## Impact
No functional change.
## Testing
Build and test ESP32 and ESP32-C3 defconfigs.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] davids5 commented on a change in pull request #3789: esp32&esp32-c3/rt_timer: Re-word some comments and fix typos
Posted by GitBox <gi...@apache.org>.
davids5 commented on a change in pull request #3789:
URL: https://github.com/apache/incubator-nuttx/pull/3789#discussion_r640053610
##########
File path: arch/risc-v/src/esp32c3/esp32c3_rt_timer.c
##########
@@ -81,13 +81,13 @@ static struct esp32c3_tim_dev_s *s_esp32c3_tim_dev;
* Name: start_rt_timer
*
* Description:
- * Start timer by inserting it into running list and reset hardware timer
- * alarm value if this timer in head of list.
+ * Start the timer by inserting it into the running list and reset the
+ * hardware timer alarm value if this timer in at the head of the list.
Review comment:
```suggestion
* hardware timer alarm value if this timer is at the head of the list.
```
##########
File path: arch/risc-v/src/esp32c3/esp32c3_rt_timer.c
##########
@@ -212,15 +213,13 @@ static void stop_rt_timer(FAR struct rt_timer_s *timer)
list_delete(&timer->list);
timer->state = RT_TIMER_IDLE;
- /* If timer is in in head of list */
+ /* If the timer is at the head of the list */
if (ishead)
{
- /* If list is not empty */
-
if (!list_is_empty(&s_runlist))
{
- /* Reset hardware timer alarm value to be next timer's */
+ /* Reset the hardware timer alarm value to be next timer's */
Review comment:
```suggestion
/* Reset the hardware timer alarm value to be next timer's timeout or alarm ??? */
```
##########
File path: arch/risc-v/src/esp32c3/esp32c3_rt_timer.c
##########
@@ -291,8 +290,8 @@ static void delete_rt_timer(FAR struct rt_timer_s *timer)
* Name: rt_timer_thread
*
* Description:
- * RT timer working thread, it wait for a timeout semaphore, scan
- * the timeout list and process all timers in this list.
+ * RT timer working thread, it waits for a timeout semaphore, scans
Review comment:
```suggestion
* RT timer working thread: Waits for a timeout semaphore, scans
```
##########
File path: arch/risc-v/src/esp32c3/esp32c3_rt_timer.c
##########
@@ -425,20 +420,19 @@ static int rt_timer_isr(int irq, void *context, void *arg)
ESP32C3_TIM_GETCTR(tim, &counter);
if (timer->alarm <= counter)
{
- /**
- * Remove first timer in running list and add it into
- * timeout list.
+ /* Remove the first timer in the running list and add it to
Review comment:
```suggestion
/* Remove the first timer from the running list and add it to
```
##########
File path: arch/xtensa/src/esp32/esp32_rt_timer.c
##########
@@ -80,13 +80,13 @@ static struct esp32_tim_dev_s *s_esp32_tim_dev;
* Name: start_rt_timer
*
* Description:
- * Start timer by inserting it into running list and reset hardware timer
- * alarm value if this timer in head of list.
+ * Start the timer by inserting it into the running list and reset the
+ * hardware timer alarm value if this timer in at the head of the list.
Review comment:
```suggestion
* hardware timer alarm value if this timer is at the head of the list.
```
##########
File path: arch/risc-v/src/esp32c3/esp32c3_rt_timer.c
##########
@@ -240,9 +239,9 @@ static void stop_rt_timer(FAR struct rt_timer_s *timer)
* Name: delete_rt_timer
*
* Description:
- * Delete timer by removing it from list, then set the timer's state
- * to be "RT_TIMER_DELETE", inserting into work list to let rt-timer
- * thread to delete it and free resource.
+ * Delete the timer by removing it from the list, then set the timer's
+ * state to be "RT_TIMER_DELETE", finally insert it into the work list
Review comment:
```suggestion
* state to "RT_TIMER_DELETE", and finally insert it into the work list
```
##########
File path: arch/xtensa/src/esp32/esp32_rt_timer.c
##########
@@ -239,9 +238,9 @@ static void stop_rt_timer(FAR struct rt_timer_s *timer)
* Name: delete_rt_timer
*
* Description:
- * Delete timer by removing it from list, then set the timer's state
- * to be "RT_TIMER_DELETE", inserting into work list to let rt-timer
- * thread to delete it and free resource.
+ * Delete the timer by removing it from the list, then set the timer's
+ * state to be "RT_TIMER_DELETE", finally insert it into the work list
Review comment:
```suggestion
* state to "RT_TIMER_DELETE", and finally insert it into the work list
```
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on a change in pull request #3789: esp32&esp32-c3/rt_timer: Re-word some comments and fix typos
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #3789:
URL: https://github.com/apache/incubator-nuttx/pull/3789#discussion_r640086570
##########
File path: arch/risc-v/src/esp32c3/esp32c3_rt_timer.c
##########
@@ -212,15 +213,13 @@ static void stop_rt_timer(FAR struct rt_timer_s *timer)
list_delete(&timer->list);
timer->state = RT_TIMER_IDLE;
- /* If timer is in in head of list */
+ /* If the timer is at the head of the list */
if (ishead)
{
- /* If list is not empty */
-
if (!list_is_empty(&s_runlist))
{
- /* Reset hardware timer alarm value to be next timer's */
+ /* Reset the hardware timer alarm value to be next timer's */
Review comment:
I believe the current writing is correct, but here is a suggestion for a (maybe) more direct explanation:
```suggestion
/* Set the value from next timer as the new hardware timer alarm value */
```
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Ouss4 commented on pull request #3789: esp32&esp32-c3/rt_timer: Re-word some comments and fix typos
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on pull request #3789:
URL: https://github.com/apache/incubator-nuttx/pull/3789#issuecomment-849170775
@gustavonihei @davids5 thanks for your review, I applied all the suggestions.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on a change in pull request #3789: esp32&esp32-c3/rt_timer: Re-word some comments and fix typos
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #3789:
URL: https://github.com/apache/incubator-nuttx/pull/3789#discussion_r640127362
##########
File path: arch/xtensa/src/esp32/esp32_rt_timer.c
##########
@@ -397,13 +392,13 @@ static int rt_timer_isr(int irq, void *context, void *arg)
ESP32_TIM_ACKINT(tim);
- /* Wake up thread to process timeout timers */
+ /* Wake up the thread to process timeout timers */
Review comment:
```suggestion
/* Wake up the thread to process timed-out timers */
```
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on a change in pull request #3789: esp32&esp32-c3/rt_timer: Re-word some comments and fix typos
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #3789:
URL: https://github.com/apache/incubator-nuttx/pull/3789#discussion_r640243783
##########
File path: arch/xtensa/src/esp32/esp32_rt_timer.c
##########
@@ -302,7 +303,7 @@ static int rt_timer_thread(int argc, FAR char *argv[])
while (1)
{
- /* Waiting for timers timeout */
+ /* Waiting for the timers timeouts */
Review comment:
```suggestion
/* Waiting for all timers to time out */
```
Suggestion to apply the same modification from other file.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on a change in pull request #3789: esp32&esp32-c3/rt_timer: Re-word some comments and fix typos
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #3789:
URL: https://github.com/apache/incubator-nuttx/pull/3789#discussion_r640128376
##########
File path: arch/risc-v/src/esp32c3/esp32c3_rt_timer.c
##########
@@ -312,7 +311,7 @@ static int rt_timer_thread(int argc, FAR char *argv[])
while (1)
{
- /* Waiting for timers timeout */
+ /* Waiting for the timers timeout */
Review comment:
```suggestion
/* Waiting for all timers to time out */
```
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on a change in pull request #3789: esp32&esp32-c3/rt_timer: Re-word some comments and fix typos
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #3789:
URL: https://github.com/apache/incubator-nuttx/pull/3789#discussion_r640244190
##########
File path: arch/xtensa/src/esp32/esp32_rt_timer.c
##########
@@ -281,8 +282,8 @@ static void delete_rt_timer(FAR struct rt_timer_s *timer)
* Name: rt_timer_thread
*
* Description:
- * RT timer working thread, it wait for a timeout semaphore, scan
- * the timeout list and process all timers in this list.
+ * RT timer working thread, it waits for a timeout semaphore, scans
Review comment:
```suggestion
* RT timer working thread: Waits for a timeout semaphore, scans
```
Copying the modification from the other file.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] davids5 merged pull request #3789: esp32&esp32-c3/rt_timer: Re-word some comments and fix typos
Posted by GitBox <gi...@apache.org>.
davids5 merged pull request #3789:
URL: https://github.com/apache/incubator-nuttx/pull/3789
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on a change in pull request #3789: esp32&esp32-c3/rt_timer: Re-word some comments and fix typos
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #3789:
URL: https://github.com/apache/incubator-nuttx/pull/3789#discussion_r640127470
##########
File path: arch/risc-v/src/esp32c3/esp32c3_rt_timer.c
##########
@@ -451,14 +445,14 @@ static int rt_timer_isr(int irq, void *context, void *arg)
}
}
- /* If there is timer in list, alarm should be enable */
+ /* If there is a timer in the list, the alarm should be enabled */
ESP32C3_TIM_SETALRM(tim, true);
}
if (wake)
{
- /* Wake up thread to process timeout timers */
+ /* Wake up the thread to process timeout timers */
Review comment:
```suggestion
/* Wake up the thread to process timed-out timers */
```
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on a change in pull request #3789: esp32&esp32-c3/rt_timer: Re-word some comments and fix typos
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #3789:
URL: https://github.com/apache/incubator-nuttx/pull/3789#discussion_r640083321
##########
File path: arch/risc-v/src/esp32c3/esp32c3_rt_timer.c
##########
@@ -187,18 +189,17 @@ static void stop_rt_timer(FAR struct rt_timer_s *timer)
flags = enter_critical_section();
- /**
- * Function "start" can set timer to be repeat, and function "stop"
- * should remove this feature although it is not in ready state.
+ /* "start" function can set the timer's repeat flag, and function "stop"
Review comment:
```suggestion
/* "start" function can set the timer's repeat flag, and "stop" function
```
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org