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

Use real headers and structures to define tables #17

Closed
skliper opened this issue Apr 22, 2022 · 2 comments · Fixed by #35
Closed

Use real headers and structures to define tables #17

skliper opened this issue Apr 22, 2022 · 2 comments · Fixed by #35
Assignees
Milestone

Comments

@skliper
Copy link
Contributor

skliper commented Apr 22, 2022

RTS uses a relative time tag that is 32 bit native endian. Current definition of tables uses a uint16 array which makes the table endian specific.

Could instead define a structure with the real contents (headers and commands), union with an array to make it the right size, then use designated initializers. This would allow the compiler to handle the endianness.

Imported from GSFCCFS-1841

skliper added a commit to skliper/SC that referenced this issue Jun 7, 2022
skliper added a commit to skliper/SC that referenced this issue Jun 7, 2022
skliper added a commit to skliper/SC that referenced this issue Nov 9, 2022
skliper added a commit to skliper/SC that referenced this issue Nov 10, 2022
@jphickey
Copy link
Contributor

Just discovered this one, its a real problem. The tables are defined as an array of uint16 values, but read by the FSW as an instance of SC_RtsEntryHeader_t as is done here:

SC_ComputeAbsTime(((SC_RtsEntryHeader_t *)SC_OperData.RtsTblAddr[RtsIndex])->TimeTag);

But the "TimeTag" value here is a uint32, not a uint16:

typedef uint32 SC_RelTimeTag_t;

Therefore, on a little endian machine, if the TimeTag is not 0, the value is incorrect. The default RTS examples (e.g. rts001, rts002) both use a value of 0 for the TimeTag, so the bug gets hidden - it never attempts to send the other 2 commands in the RTS, that have nonzero values here.

I changed CMD1_TIME from 0 to 1, and sure enough, the SC_ComputeAbsTime is called with a value of 0x1000 (65536), rather than 1, because it was incorrectly cast.

It looks like ATS was updated to split the time tag into a 16-bit MS and LS word (a hack, but will at least get the logical value to come through), but RTS was not updated in the same manner.

@skliper
Copy link
Contributor Author

skliper commented Dec 13, 2022

@jphickey I've got a PR that was just waiting on something to switch to using the actual structures. Whatever I was waiting on is probably in by now if you want me to move this forward. It doesn't fix how RTS/ATS define time differently... but at least it's endian agnostic.

@dmknutsen dmknutsen self-assigned this Dec 14, 2022
skliper added a commit to skliper/SC that referenced this issue Dec 20, 2022
dzbaker added a commit that referenced this issue Jan 5, 2023
Fix #17, Use real message types in tables
@chillfig chillfig added this to the Draco milestone Feb 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants