Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Local #defines in cf_cmd.c used as special values in command processing (w/ repeat dfn in tests) #263

Closed
3 tasks done
skliper opened this issue Jun 16, 2022 · 1 comment · Fixed by #327
Closed
3 tasks done
Assignees
Milestone

Comments

@skliper
Copy link
Contributor

skliper commented Jun 16, 2022

Checklist (Please check before submitting)

  • I reviewed the Contributing Guide.
  • I reviewed the CF README.md file to see if the feature is in the major future work.
  • I performed a cursory search to see if the feature request is relevant, not redundant, nor in conflict with other tickets.

Is your feature request related to a problem? Please describe.
ALL_CHANNELS, ALL_POLLDIRS, COMPOUND_KEY used in command processing shouldn't be defined in a *.c where it's inaccessible and somewhat hidden. The names aren't great either, better to prefix w/ app.

CF/fsw/src/cf_cmd.c

Lines 41 to 43 in 593d61a

#define ALL_CHANNELS (255)
#define ALL_POLLDIRS (ALL_CHANNELS)
#define COMPOUND_KEY (254)

Describe the solution you'd like
Move into cfe_msg.h and rename as CF_ALL_CHANNELS and similar, which would allow removal from cf_test_utils.h:

#define ALL_CHANNELS 255

Describe alternatives you've considered
None

Additional context
None

Requester Info
Jacob Hageman - NASA/GSFC

@skliper skliper changed the title Local #defines in cf_cmd.c but used as special values in command processing (w/ repeat dfn in tests) Local #defines in cf_cmd.c used as special values in command processing (w/ repeat dfn in tests) Jun 16, 2022
@skliper
Copy link
Contributor Author

skliper commented Jul 25, 2022

Slightly different pattern but similar issue with CF_CmdReset, the following for counters_command, counters_fault, counters_up, counters_down:

CF/fsw/src/cf_cmd.c

Lines 70 to 73 in 0774746

static const int counters_command = 1;
static const int counters_fault = 2;
static const int counters_up = 3;
static const int counters_down = 4;

CF_CmdWriteQueue for type_up, type_down, q_pend, q_active, q_history, q_all:

CF/fsw/src/cf_cmd.c

Lines 832 to 837 in 0774746

static const int type_up = 1;
static const int type_down = 2;
static const int q_pend = 0;
static const int q_active = 1;
static const int q_history = 2;
static const int q_all = 3;

Forces duplicate dfns, fragile.

@havencarlson havencarlson self-assigned this Sep 28, 2022
havencarlson added a commit to havencarlson/CF that referenced this issue Sep 28, 2022
havencarlson added a commit to havencarlson/CF that referenced this issue Sep 28, 2022
dzbaker added a commit that referenced this issue Oct 3, 2022
Fix #263, define special values in cmd processing as enums
@dmknutsen dmknutsen added this to the Draco milestone Feb 6, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants