Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sensor: remove noop 1 Hz timer #1702

Merged
merged 1 commit into from
Apr 2, 2024
Merged

Conversation

cbiffle
Copy link
Collaborator

@cbiffle cbiffle commented Apr 1, 2024

task/sensor was configuring a timer to wake it every 1 Hz. Each time it woke up, it would snooze its alarm clock and go back to sleep.

While I definitely sympathize with that desire, this is essentially dead code that looks like a bug, so this commit removes it. I'm not sure what the history here is.

In addition to removing noise from the code, this knocks 114 bytes off the sensor task text size.

@cbiffle cbiffle requested a review from bcantrill April 1, 2024 21:27

// N.B. if you are staring at this macro thinking that it looks like it
// doesn't do anything and might be obsolescent, the key is the :upper. This
// macro exists exclusively to uppercase the field names below.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

(this macro confused me so I tossed in this documentation of it while I was here.)

@cbiffle cbiffle force-pushed the cbiffle/remove-noop-sensor-timer branch 2 times, most recently from 9569fc6 to 6637e13 Compare April 1, 2024 21:42
Copy link
Member

@hawkw hawkw left a comment

Choose a reason for hiding this comment

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

looks good to me (although it would be nice to hear from Bryan about the original motivation for this)

Copy link
Collaborator

@bcantrill bcantrill left a comment

Choose a reason for hiding this comment

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

Sadly, I think that this was born vestigial. :( Definitely rip it out, with my apologies!

task/sensor was configuring a timer to wake it every 1 Hz. Each time it
woke up, it would snooze its alarm clock and go back to sleep.

While I definitely sympathize with that desire, this is essentially dead
code that looks like a bug, so this commit removes it. I'm not sure what
the history here is.

In addition to removing noise from the code, this knocks 114 bytes off
the sensor task text size.
@cbiffle cbiffle force-pushed the cbiffle/remove-noop-sensor-timer branch from 6637e13 to 2e24fbe Compare April 2, 2024 23:09
@cbiffle cbiffle enabled auto-merge (rebase) April 2, 2024 23:09
@cbiffle cbiffle merged commit 36aaed8 into master Apr 2, 2024
103 checks passed
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.

3 participants