You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by "jlaitine (via GitHub)" <gi...@apache.org> on 2023/01/31 09:29:39 UTC

[GitHub] [nuttx] jlaitine commented on a diff in pull request #8371: Change sec2tick macros to round up

jlaitine commented on code in PR #8371:
URL: https://github.com/apache/nuttx/pull/8371#discussion_r1091664006


##########
include/nuttx/clock.h:
##########
@@ -145,41 +145,49 @@
  * preferred for that reason (at the risk of overflow)
  */
 
-#define TICK_PER_HOUR         (USEC_PER_HOUR / USEC_PER_TICK)            /* Truncates! */
-#define TICK_PER_MIN          (USEC_PER_MIN  / USEC_PER_TICK)            /* Truncates! */
-#define TICK_PER_SEC          (USEC_PER_SEC  / USEC_PER_TICK)            /* Truncates! */
-#define TICK_PER_MSEC         (USEC_PER_MSEC / USEC_PER_TICK)            /* Truncates! */
-#define TICK_PER_DSEC         (USEC_PER_DSEC / USEC_PER_TICK)            /* Truncates! */
-#define TICK_PER_HSEC         (USEC_PER_HSEC / USEC_PER_TICK)            /* Truncates! */
+/* TICK_PER_* truncates! */
 
-#define MSEC_PER_TICK         (USEC_PER_TICK / USEC_PER_MSEC)            /* Truncates! */
-#define NSEC_PER_TICK         (USEC_PER_TICK * NSEC_PER_USEC)            /* Exact */
+#define TICK_PER_HOUR         (USEC_PER_HOUR / USEC_PER_TICK)

Review Comment:
   Intuitively I'd say no:
   
   - The intended use for those macros is different than of those that I was now changing
   - There is a comment warning about these macros a bit above, describing the truncation/inaccuracy
   - They haven't been used in many places (basically only TICK_PER_SEC is used) and in most (normal) cases it wouldn't make any difference either
   - The TICK_PER_SEC is used in clock_ticks2time.c in a similar way as some other OSs do tick->timespec conversion, so it is currenly expecting it to truncate.
   - They are also not directly related to the SEC2TICK macros, as due to rounding it is not anyway possible to do 1<->1 conversion between those, regardless of what rounding is used
   
   So I wouldn't touch them now unless there is a good reason to do so (?)
   
   



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