Avoid setting AP_FENCE_ENABLED to 2

[shiv-tyagi]

Currently, we suggest that AC_FENCE_ENABLED can be set to 0, 1 or 2, which is against the pattern followed throughout the codebase. In the AP_*_ENABLED world, something should either be ENABLED or DISABLED, nothing in between.

Setting AC_FENCE_ENABLED to 2 just brings only one additional change in behavior compared to setting it to 1, which is - it compiles code with some dummy methods in AC_Fence for tracker/blimp.

We already have AC_FENCE_DUMMY_METHODS_ENABLED for that and a user wanting to enable/disable dummy methods, should do so by directly setting AC_FENCE_DUMMY_METHODS_ENABLED, NOT by setting AC_FENCE_ENABLED to 2 (and relying on that to set AC_FENCE_DUMMY_METHODS_ENABLED to 1 in turn).

I caught this while working on a Custom Build Server enhancement and this really breaks things there.

[peterbarker]

I think you're missing the points of that == 2 thing.

------  -----  ----------  ------  ----  -----  -----  ---
Board   blimp  bootloader  copter  heli  plane  rover  sub
CUAVv5  2632   *           *       *     *      *      *
------  -----  ----------  ------  ----  -----  -----  ---
pbarker@crun:~/rc/ardupilot((HEAD detached at 939d3e7ce4d))$ 

tracker would also get the same size bigger.

As mentioned elsewhere, I think you could propose removing the == 2 stuff (and the dummy methods) entirely and it would get up as a PR.

[shiv-tyagi]

@peterbarker The AC_FENCE_DUMMY_METHODS_ENABLED was defined at two places instead of one and I missed fixing it at one place. It is fixed now and results in zero size change.

Binary Name      Text [B]     Data [B]     BSS (B)      Total Flash Change [B] (%)      Flash Free After PR (B)
---------------  -----------  -----------  -----------  ----------------------------  -------------------------
arducopter-heli  0 (0.0000%)  0 (0.0000%)  0 (0.0000%)  0 (0.0000%)                                       66028
arducopter       0 (0.0000%)  0 (0.0000%)  0 (0.0000%)  0 (0.0000%)                                       69420
blimp            0 (0.0000%)  0 (0.0000%)  0 (0.0000%)  0 (0.0000%)                                      293816
ardusub          0 (0.0000%)  0 (0.0000%)  0 (0.0000%)  0 (0.0000%)                                      140864
arduplane        0 (0.0000%)  0 (0.0000%)  0 (0.0000%)  0 (0.0000%)                                       19036
ardurover        0 (0.0000%)  0 (0.0000%)  0 (0.0000%)  0 (0.0000%)                                      158704
antennatracker   0 (0.0000%)  0 (0.0000%)  0 (0.0000%)  0 (0.0000%)                                      311184

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[peterbarker]

I like this. But I think we should also look at just eliminating the dummy method stuff entirely for Fence!

[peterbarker]

OK, I think we need one more patch in here. If a user currently has AC_FENCE_ENABLED set to 2 then we should throw a #error.

Should also ping @IamPete1 as he put this stuff in.

Again, I'm thinking we can live with Blimp and Tracker having fence simply on by default. But that step is easier past this PR anyway as we just remove anything to do with the dummy methods.

[shiv-tyagi]

This is going because:-

1. we do not wrap the call to this method with this define.
2. the non-dummy implementation for this method is also not wrapped under this define

[shiv-tyagi]

Binary Name      Text [B]     Data [B]     BSS (B)      Total Flash Change [B] (%)      Flash Free After PR (B)
---------------  -----------  -----------  -----------  ----------------------------  -------------------------
arducopter-heli  0 (0.0000%)  0 (0.0000%)  0 (0.0000%)  0 (0.0000%)                                       65276
arducopter       0 (0.0000%)  0 (0.0000%)  0 (0.0000%)  0 (0.0000%)                                       68628
blimp            0 (0.0000%)  0 (0.0000%)  0 (0.0000%)  0 (0.0000%)                                      293600
ardusub          0 (0.0000%)  0 (0.0000%)  0 (0.0000%)  0 (0.0000%)                                      139696
arduplane        0 (0.0000%)  0 (0.0000%)  0 (0.0000%)  0 (0.0000%)                                       18092
ardurover        0 (0.0000%)  0 (0.0000%)  0 (0.0000%)  0 (0.0000%)                                      158496
antennatracker   0 (0.0000%)  0 (0.0000%)  0 (0.0000%)  0 (0.0000%)                                      310976

No compiler output change for the changes included in this PR.

[IamPete1]

We did originally start this pattern for the build server. The ides was that 0 would remove the feature, and 1 would enable it. 2 would then be the correct default for the vehicle type.

I have no problem with this here in Fence because no one would ever need fence on tracker or blimp. However, we use the same pattern for AIS and Temperature sensor. Its perfectly reasonable to want AIS outside of rover, and it will still work correctly for reporting.

How would the build server handle this going forward?