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/10/18 13:52:52 UTC

[GitHub] [incubator-nuttx] fjpanag opened a new pull request, #7349: gmtimer: Fixed range of tm_yday.

fjpanag opened a new pull request, #7349:
URL: https://github.com/apache/incubator-nuttx/pull/7349

   ## Summary
   
   `tm_yday` is zero-based, and has a range of 0-365.
   
   Previously the returned value had the range of 1-366 which is wrong. This PR fixes it.
   
   ## Impact
   
   Bug fix.
   
   ## Testing
   
   Tested on simulator, it now returns the correct value for the current day (as of writing this, 290).
   


-- 
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] fjpanag commented on a diff in pull request #7349: gmtimer: Fixed range of tm_yday.

Posted by GitBox <gi...@apache.org>.
fjpanag commented on code in PR #7349:
URL: https://github.com/apache/incubator-nuttx/pull/7349#discussion_r998800825


##########
libs/libc/time/lib_gmtimer.c:
##########
@@ -336,7 +336,7 @@ FAR struct tm *gmtime_r(FAR const time_t *timep, FAR struct tm *result)
   result->tm_wday   = clock_dayoftheweek(day, month, year);
   result->tm_yday   = day +
                       clock_daysbeforemonth(result->tm_mon,
-                                            clock_isleapyear(year));
+                                            clock_isleapyear(year)) - 1;

Review Comment:
   Yep, sorry. I did it in two steps and I didn't notice it.  
   Fixed.



-- 
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 #7349: gmtimer: Fixed range of tm_yday.

Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #7349:
URL: https://github.com/apache/incubator-nuttx/pull/7349#discussion_r998648301


##########
libs/libc/time/lib_gmtimer.c:
##########
@@ -336,7 +336,7 @@ FAR struct tm *gmtime_r(FAR const time_t *timep, FAR struct tm *result)
   result->tm_wday   = clock_dayoftheweek(day, month, year);
   result->tm_yday   = day +
                       clock_daysbeforemonth(result->tm_mon,
-                                            clock_isleapyear(year));
+                                            clock_isleapyear(year)) - 1;

Review Comment:
   maybe `day - 1 + clock_daysbeforemonth()` just for the style consistency? Or change to `tp->tm_mday + clock_daysbeforemonth() - 1` in other places?



-- 
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] fjpanag commented on pull request #7349: gmtimer: Fixed range of tm_yday.

Posted by GitBox <gi...@apache.org>.
fjpanag commented on PR #7349:
URL: https://github.com/apache/incubator-nuttx/pull/7349#issuecomment-1282455438

   Indeed the STM32 drivers need to be fixed too.  
   I added a new commit with these changes, but I haven't tried this on hardware yet.
   
   I expect some tests in a few hours on an STM32F427.


-- 
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 #7349: gmtimer: Fixed range of tm_yday.

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged PR #7349:
URL: https://github.com/apache/incubator-nuttx/pull/7349


-- 
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] fjpanag commented on pull request #7349: gmtimer: Fixed range of tm_yday.

Posted by GitBox <gi...@apache.org>.
fjpanag commented on PR #7349:
URL: https://github.com/apache/incubator-nuttx/pull/7349#issuecomment-1282432615

   I see that the same calculation is used in the various implementations of `up_rtc_getdatetime()` for the STM32.
   
   I am investigating now whether this change needs to be applied there too.


-- 
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