nasa / SC

The Core Flight System (cFS) Stored Commands (SC) application.
Apache License 2.0
38 stars 20 forks source link

Inconsistent use of `SC_LAST_RTS_WITH_EVENTS` #138

Closed skliper closed 8 months ago

skliper commented 8 months ago

Checklist (Please check before submitting)

Describe the bug SC_LAST_RTS_WITH_EVENTS is documented like an index (1 based), but converted from 0 based in tests.

Documented as 1 based: https://github.com/nasa/SC/blob/29f7badc4dcf3e45d29e25b04c5f58df4de78227/config/default_sc_internal_cfg.h#L139-L142

Verified as 1 based: https://github.com/nasa/SC/blob/29f7badc4dcf3e45d29e25b04c5f58df4de78227/fsw/src/sc_verify.h#L108-L114

Compared against 1 based number here: https://github.com/nasa/SC/blob/29f7badc4dcf3e45d29e25b04c5f58df4de78227/fsw/src/sc_rtsrq.c#L111

Correctly converted to an index here: https://github.com/nasa/SC/blob/29f7badc4dcf3e45d29e25b04c5f58df4de78227/unit-test/sc_state_tests.c#L453

Questionable use here since CFE_MSG_Init expects a size: https://github.com/nasa/SC/blob/29f7badc4dcf3e45d29e25b04c5f58df4de78227/unit-test/sc_state_tests.c#L471

BROKEN references here, used as 0 based: https://github.com/nasa/SC/blob/29f7badc4dcf3e45d29e25b04c5f58df4de78227/unit-test/sc_state_tests.c#L499 https://github.com/nasa/SC/blob/29f7badc4dcf3e45d29e25b04c5f58df4de78227/unit-test/sc_state_tests.c#L586

These tests will probably fail if SC_LAST_RTS_WITH_EVENTS set to SC_NUMBER_OF_RTS - 1, should work correctly if 1 based. https://github.com/nasa/SC/blob/29f7badc4dcf3e45d29e25b04c5f58df4de78227/unit-test/sc_state_tests.c#L788-L794

To Reproduce Set SC_LAST_RTS_WITH_EVENTS to SC_NUMBER_OF_RTS as documented in the config header, fails at compile time Note if set to SC_NUMBER_OF_RTS - 1, it doesn't look like it will report the last RTS events

Expected behavior Setting SC_LAST_RTS_WITH_EVENTS to SC_NUMBER_OF_RTS should send event for all RTS's

Code snips See above

System observed on: all

Additional context None

Reporter Info Jacob Hageman/NASA GSFC

Ping @jphickey