Fix #105, remove SC_ENABLE_GROUP_COMMANDS option

[jphickey]

Checklist (Please check before submitting)

• I reviewed the Contributing Guide.
• I signed and emailed the appropriate Contributor License Agreement to [email protected] and copied [email protected].

Describe the contribution
This was always set true and the software has not been validated with the option set to false. Turning this off only removed a few small functions and the complexity (and violation of coding standard) does not justify it as an option.

Fixes #105

Testing performed
Build and run all tests

Expected behavior changes
None, assuming code was always built with this option set true anyway

System(s) tested on
Debian

Contributor Info - All information REQUIRED for consideration of pull request
Joseph Hickey, Vantage Systems, Inc.

[jphickey]

Note - I also ran a quick test to see what happened if this option was set false - and it turns out the unit test code doesn't even build that way, its obviously been broken for a while:

apps/sc/unit-test/sc_rtsrq_tests.c: In function ‘SC_StartRtsGrpCmd_Test_Nominal’:
apps/sc/unit-test/sc_rtsrq_tests.c:270:15: error: ‘UT_CmdBuf_t’ has no member named ‘RtsGrpCmd’; did you mean ‘RtsCmd’?
  270 |     UT_CmdBuf.RtsGrpCmd.Payload.FirstRtsId = 1;
      |               ^~~~~~~~~
      |               RtsCmd
apps/sc/unit-test/sc_rtsrq_tests.c:271:15: error: ‘UT_CmdBuf_t’ has no member named ‘RtsGrpCmd’; did you mean ‘RtsCmd’?
  271 |     UT_CmdBuf.RtsGrpCmd.Payload.LastRtsId  = 1;
      |               ^~~~~~~~~
      |               RtsCmd

This is why we avoid conditional code.

[jphickey]

Comparison of code size with option set true:

   text    data     bss     dec     hex filename
  48414    1336   16928   66678   10476 ./exe/cpu1/cf/sc.so

And option set false (unit test disabled):

   text    data     bss     dec     hex filename
  44820    1304   16928   63052    f64c ./exe/cpu1/cf/sc.so

So we are talking about 3.6kb of code size and a few bytes of data saved by disabling this feature. With this PR this will be permanently enabled.

[skliper]

Looked up the requirements for group commands and they aren't marked optional (although they don't follow best practices for avoiding "compound" requirements). Note I didn't find requirements for the enable/disable group commands. See #107.

[skliper]

@jphickey - could you add the settings you used (optimization level, did you strip debug?) and platform for the code size results?

[jphickey]

Note - Force pushes above are just to tweak UT stubs to avoid a merge conflict with my next PR.

[jphickey]

@skliper - the size test was done with a vanilla Linux debug build using gcc 12.

If I do a "release" build instead (thus -O3 optimization and binaries w/o debug info), the text numbers get a bit smaller:

Enabled:

   text    data     bss     dec     hex filename
  42047    1336   16920   60303    eb8f ./cf/sc.so

Disabled:

   text    data     bss     dec     hex filename
  39197    1304   16920   57421    e04d ./cf/sc.so

If the code size was the impetus for making this compile-time optional, then there are other ways to make the code smaller that don't create a validation issue.

[skliper]

@jphickey Agree completely w/ the change and general move away from compile time ifdefs, just helps to know what settings were used when sizes are reported.