Provide consistency in setting of prevRingTime for alarms when changing ref time #1290
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mgduda
force-pushed
the
framework/adjust_alarm_prevRingTime
branch
from
02292ee to
612799b
Compare
This commit adds unit tests for the mpas_adjust_alarm_to_reference_time routine to the test_core_timekeeping_tests module in the test core. Implied in these tests is the requirement that, after a call to mpas_adjust_alarm_to_reference_time, the prevRingTime of the alarm is set to a value that ensures that an immediately subsequent call to mpas_is_alarm_ringing will show that the alarm is ringing. In order to get more verbose printout, including the internal state of alarms, the MPAS_ADJUST_ALARM_VERBOSE macro can be modfied near the top of the test_core_timekeeping_tests module so that its argument is not turned into a comment.
…ng ref time
This commit aims to provide consistency in the ringing behavior of alarms after
a call to mpas_adjust_alarm_to_reference_time to alter the reference time for a
recurring alarm.
For simplicity of discussion, consider only a clock that is running forward in
time. The objective of the mpas_adjust_alarm_to_reference_time routine is to set
the prevRingTime (previous ring time) of a recurring alarm to (1) a time that
differs from the specified reference time by an integer multiple of the alarm
interval; and (2) the latest such time that is not after the current time on the
clock.
Prior to this commit, the logic in the mpas_adjust_alarm_to_reference_time
routine resulted in one of two outcomes:
(1) If the difference between the reference time and the current time is
divisible by the alarm interval, the prevRingTime becomes the current time, as
illustrated below.
ring ring ring ring
v v v v
-----------------------------------------------------------
^
now
prevRingTime
(2) Otherwise, the prevRingTime becomes the latest time before the current time
that lies on an integer multiple of the alarm interval away from the reference
time, as illustrated below.
ring ring ring ring
v v v v
-----------------------------------------------------------
^ ^
prevRingTime now
To determine whether a recurring alarm is ringing, the alarm's interval is added
to the alarm's prevRingTime. If the result is on or before the current time, the
alarm is ringing; otherwise, if the result is later than the current time, the
alarm is not ringing. As a consequence, outcome (1) from the
mpas_adjust_alarm_to_reference_time leads to an alarm that is ringing after the
call to the routine, while outcome(2) leads to an alarm that is not yet ringing.
In order to rectify the inconsistency in whether an alarm is ringing depending
on where its reference time is set relative to the current time, the
prevRingTime for an alarm is always set to be at least one full alarm interval
before the current time. Whether the difference between the current time and the
reference time is evenly divisible by the alarm's ring interval or not, a query
of the alarm's status immediately following the call to
mpas_adjust_alarm_to_reference_time will always show that it is ringing.
This commit makes changes to the logic for both a forward and a backward running
clock in the mpas_adjust_alarm_to_reference_time routine, although it appears
that no code actually makes use of a backward running clock at present.
Note: At present, the logic is not quite as elegant as one might expect it
should be. In future, if the interval_division routine handled negative
intervals like the analog of
(-2.75 % 1.0) => 0.25
then there may be no need for if-tests within blocks for each clock direction.
mgduda
force-pushed
the
framework/adjust_alarm_prevRingTime
branch
from
612799b to
f6039c4
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR aims to provide consistency in the ringing behavior of alarms after a call to
mpas_adjust_alarm_to_reference_timeto alter the reference time for a recurring alarm.For simplicity of discussion, consider only a clock that is running forward in time. The objective of the
mpas_adjust_alarm_to_reference_timeroutine is to set theprevRingTime(previous ring time) of a recurring alarm to (1) a time that differs from the specified reference time by an integer multiple of the alarm interval; and (2) the latest such time that is not after the current time on the clock.Prior to this commit, the logic in the
mpas_adjust_alarm_to_reference_timeroutine resulted in one of two outcomes:(1) If the difference between the reference time and the current time is divisible by the alarm interval, the
prevRingTimebecomes the current time, as illustrated below.(2) Otherwise, the
prevRingTimebecomes the latest time before the current time that lies on an integer multiple of the alarm interval away from the reference time, as illustrated below.To determine whether a recurring alarm is ringing, the alarm's interval is added to the alarm's
prevRingTime. If the result is on or before the current time, the alarm is ringing; otherwise, if the result is later than the current time, the alarm is not ringing. As a consequence, outcome (1) from thempas_adjust_alarm_to_reference_timeleads to an alarm that is ringing after the call to the routine, while outcome(2) leads to an alarm that is not yet ringing.In order to rectify the inconsistency in whether an alarm is ringing depending on where its reference time is set relative to the current time, the
prevRingTimefor an alarm is always set to be at least one full alarm interval before the current time. Whether the difference between the current time and the reference time is evenly divisible by the alarm's ring interval or not, a query of the alarm's status will always show that it is ringing.This PR makes changes to the logic for both a forward and a backward running clock in the
mpas_adjust_alarm_to_reference_time, although it appears that no code actually makes use of a backward running clock at present.