Skip to content

Edited macro for ms_to_ticks#19

Merged
bjackson312006 merged 1 commit into
mainfrom
ms-to-ticks
Aug 1, 2025
Merged

Edited macro for ms_to_ticks#19
bjackson312006 merged 1 commit into
mainfrom
ms-to-ticks

Conversation

@bjackson312006

Copy link
Copy Markdown
Collaborator

No description provided.

@bjackson312006 bjackson312006 self-assigned this Aug 1, 2025
@bjackson312006
bjackson312006 requested a review from Copilot August 1, 2025 18:02

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR modifies the MS_TO_TICKS macro to prevent truncation issues when converting milliseconds to timer ticks. The change adds 999 to the numerator before division to implement ceiling division, ensuring that small millisecond values don't get truncated to zero ticks.

  • Updated the MS_TO_TICKS macro to use ceiling division instead of floor division
  • Removed the TODO comment about potential truncation issues

Comment thread Core/Inc/u_config.h

/* Time and tick conversions */
#define MS_TO_TICKS(ms) ((ms) * TX_TIMER_TICKS_PER_SECOND / 1000) // u_TODO - i think this truncates if ms < 10. dunno if that will be an issue
#define MS_TO_TICKS(ms) (((ms) * TX_TIMER_TICKS_PER_SECOND + 999) / 1000) //

Copilot AI Aug 1, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ceiling division implementation is incorrect. Adding 999 assumes the divisor is always 1000, but the actual divisor is 1000. For proper ceiling division, use (((ms) * TX_TIMER_TICKS_PER_SECOND + 1000 - 1) / 1000) or the more general form (((ms) * TX_TIMER_TICKS_PER_SECOND + 999) / 1000) only if you're certain the divisor is exactly 1000.

Suggested change
#define MS_TO_TICKS(ms) (((ms) * TX_TIMER_TICKS_PER_SECOND + 999) / 1000) //
#define MS_TO_TICKS(ms) (((ms) * TX_TIMER_TICKS_PER_SECOND + TX_TIMER_TICKS_PER_SECOND - 1) / TX_TIMER_TICKS_PER_SECOND) //

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

brain rot

Comment thread Core/Inc/u_config.h

/* Time and tick conversions */
#define MS_TO_TICKS(ms) ((ms) * TX_TIMER_TICKS_PER_SECOND / 1000) // u_TODO - i think this truncates if ms < 10. dunno if that will be an issue
#define MS_TO_TICKS(ms) (((ms) * TX_TIMER_TICKS_PER_SECOND + 999) / 1000) //

Copilot AI Aug 1, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The trailing comment is empty and should either be removed or contain meaningful documentation explaining the ceiling division behavior.

Suggested change
#define MS_TO_TICKS(ms) (((ms) * TX_TIMER_TICKS_PER_SECOND + 999) / 1000) //
#define MS_TO_TICKS(ms) (((ms) * TX_TIMER_TICKS_PER_SECOND + 999) / 1000) /* Converts milliseconds to ticks, rounding up to ensure partial ticks are counted (ceiling division). */

Copilot uses AI. Check for mistakes.
@bjackson312006
bjackson312006 merged commit 988fdb9 into main Aug 1, 2025
1 check passed
@bjackson312006
bjackson312006 deleted the ms-to-ticks branch August 1, 2025 18:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants