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

Update CFE to use OS time conversion/access methods #1051

Closed
jphickey opened this issue Dec 31, 2020 · 1 comment · Fixed by #1058 or #1088
Closed

Update CFE to use OS time conversion/access methods #1051

jphickey opened this issue Dec 31, 2020 · 1 comment · Fixed by #1058 or #1088
Assignees
Milestone

Comments

@jphickey
Copy link
Contributor

Is your feature request related to a problem? Please describe.
CFE is directly accessing specific fields within OS_time_t which will break when the struct definition changes.

Describe the solution you'd like
Instead of directly accessing the seconds and microsecs fields within OS_time_t, use the accessor functions to convert/extract the relevant info from the value instead.

Additional context
see nasa/osal#429

Requester Info
Joseph Hickey, Vantage Systems, Inc.

@jphickey jphickey self-assigned this Dec 31, 2020
@jphickey jphickey changed the title Update CFE to use OS time abstract Update CFE to use OS time conversion/access methods Dec 31, 2020
@jphickey
Copy link
Contributor Author

Also worth noting here that the CFE_TIME_Micro2SubSecs and CFE_TIME_Sub2MicroSecs are fairly broken. I had never really tested/looked at them with any level of detail until now.

There are plenty of values that do not convert to the "ideal" value. For instance the CFE_TIME unit test uses a microsecond value of "567890", for which CFE_TIME_Micro2SubSecs() returns a value of 0x91614000 ... but the closest value by my calculation 0x91613d32 ... the former converts back to 567890.167236us, but the latter converts back to 567890.0000064us - so much closer to the real value.

Of course in this case when CFE_TIME_Sub2MicroSecs rounds it back to an integer, it returns the same original value (567890) but this isn't always the case.

EXAMPLE: At a value of 568017us, CFE_TIME_Micro2SubSecs() returns a value of 0x91699000 (568017.00592) ... the CFE_TIME_Sub2MicroSecs function then converts this back to 568016, not 568017 as was the original value. (FWIW, by my calculation closest/correct value should actually be 0x91698fe7 here, which is 568017.0001us).

My suggestion is to provide OSAL functions that actually implement this properly.

@skliper skliper added this to the 7.0.0 milestone Jan 4, 2021
jphickey added a commit to jphickey/cFE that referenced this issue Jan 4, 2021
Instead of accessing OS_time_t values directly, use the
OSAL-provided conversion and access methods.  This provides
independence/abstraction from the specific OS_time_t
definition and allows OSAL to transition to a 64 bit value.
astrogeco added a commit that referenced this issue Jan 13, 2021
Fix #1051, use OSAL time conversion/access methods
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants