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.
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
Resolution of issue "Non slewing time adjustments #203" V2 #354
base: master
Are you sure you want to change the base?
Resolution of issue "Non slewing time adjustments #203" V2 #354
Changes from 2 commits
9f1e31cd18ed6d99b980cdcf91d7326d7c232670f07b4b61c70308df2aa9627693dc87d9c5cd6222b1b9485d076File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I want this to be even more strict:
While a provider cannot verify coherency of timestamps in the MDIB, for every MDS that is affected, the provider shall set pm:Clock/@ActivationState = Fail.
NOTE: If for technical reasons or risk related measures a provider cannot set a new sequence id after a non-slewing time adjustment ensued, the provider sets the clock into fail mode such that consumers know they must not trust any timestamps that are read from the provider's MDIB.
That includes setting the clock to fail even if epoch versions are used. This allows for the consumers that do not understand epoch versions to act as if the clock would not be reliable anymore. From that it follows that the epoch versioning extension is required to be uniquely identifiable by a consumer as a means to recover from a failed clock.
If a provider does not want to set its clock to Fail, it may produce a new MDIB sequence at any time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I'm not sure I completely follow this one and its interaction with other points. I think it depends on whether we put
mustUnderstand="true"on some extension in the clock descriptor, and whether we are trying to deal with participants outside SDPi. This is where I got to:If we put
mustUnderstand="true"on in the clock's descriptor for an versioning extension:sdpi:Epochs/@Versionchanging signals a abrupt time adjustment,pm:ClockState/@LastSetsignals when that occurred (in the current time-reference frame),@LastSetcan be relied on even if the consumer understands epoch versioning and the timestamps are versioned because:pm:ClockState/@ActivationState != "On"but in the above context it isn't necessary.Failfor clock failures (e.g., hardware fault) and useStndbyfor synchronization failures; maybe we don't need to separate these things though.pm:AbstractDescriptor/@SafetyClassification="MedA"(display only) might have a relaxed attitude to abrupt time jumps happy to just convey the information while aMedC(clinical function) provider might fault the whole device.If
mustUnderstandisn't true, I don't think there is anything about how a consumer should interpretClockState/@ActivationState != "On". It might surprise them but SDPi conformance testing could validate they do something sensible. Although consumers that are on-board with SDPi should know what do withmustUnderstand=true. Are we worried about non SDPi compliant consumers? This just makes me think we should havemustUnderstand=true.In conclusion, I'm not sure what to do next here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I think there is a lot of confusion going on - let my try to settle things:
Now that I am getting depper into the topic, I think we can make the extension mustUnderstand=false, because we signify "something is being wrong" by using the StdBy flag and add epoch information to allow for consumers to calculate correct times based on epoch versions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This time business is much more complicated than it looks! I intended the latest R1522 to do all the things you mentioned:
When the <<vol1_spec_sdpi_p_actor_somds_provider>> detects an abrupt time adjustment of a system clock, used in making its System Function Contribution (<<acronym_sfc>>), the <<vol1_spec_sdpi_p_actor_somds_provider>> shall either:
pm:ClockState/@ActivationStatetoStndBywhen any timestamp in a <<acronym_mdib>> version was not obtained from the time-reference frame of the active clock in the same version, orpm:ClockState/@LastSetto the earliest time that is unambiguously in the current epoch and incrementsdpi:Epochs/@Version.To break it down:
ClockState/@ActivationState, that there are one or more timestamps from a different time-reference frame in the mdib. They could be anywhere (metrics, components or extensions etc) and there is no way to tell which timestamps are trustworthy and which are not.sdpi:Epochs/@Versionchanging andpm:ClockState/@LastSet. That is a consumer can compare every timestamp (whether metric, component or extension) to the value ofpm:ClockState/@LastSet; any that are strictly less are untrustworthy. That's the case at the time of the adjustment or six months later.pm:ClockState/@LastSet).Note
the provider may generate timestamps from the current time-reference frame that are less than
pm:ClockState/@LastSet. This can happen when a non-slewing adjustment moves the clock backwards in time and the MDIB previously contained timestamps from the "future".The downside of (3) not setting the
@ActivationStateis that the consumer has to:sdpi:Epochs/@Version, andOne approach I can see providers taking with option 3 is leaving timestamps to update of their own accord. Notably, contexts may have untrustworthy timestamps until they are validated again, perhaps in a week or two when the provider is used on a new patient. It seems consumers might have a hard time figuring out what to do if
@ActivationState="StndBy"for a week or two. However, if only setting@ActivationState="StndBy"is one of the approaches we want to have here, then I think it would make sense for the versioned one to also do that.So, I'll update the text to:
When the <<vol1_spec_sdpi_p_actor_somds_provider>> detects an abrupt time adjustment of a system clock, used in making its System Function Contribution (<<acronym_sfc>>), the <<vol1_spec_sdpi_p_actor_somds_provider>> shall either:
pm:ClockState/@ActivationStatetoStndBywhen any timestamp in a <<acronym_mdib>> version was not obtained from the time-reference frame of the active clock in the same version, orpm:ClockState/@LastSetto the earliest time that is unambiguously in the current epoch and incrementsdpi:Epochs/@Versionand setpm:ClockState/@ActivationStatetoStndBywhile any timestamp in a <<acronym_mdib>> version is less thanpm:ClockState/@LastSet.I worded it slightly differently for greater clarity with the relationship to
pm:ClockState/@LastSetbut the behaviour ofpm:ClockState/@ActivationStateis identical for 2 & 3.