tim-balloon / TIMflight

Flight MCP repo
0 stars 0 forks source link

floating point in equality comparison commands.c #5

Closed evanmayer closed 2 years ago

evanmayer commented 2 years ago

Task

Floating point comparisons of (in)equality are used in command logic.

./mcp/commanding/commands.c:

      if ((CommandData.pointing_mode.mode != P_AZEL_GOTO) ||
          (CommandData.pointing_mode.X != rvalues[0]) ||
          (CommandData.pointing_mode.Y != rvalues[1])) {
        CommandData.pointing_mode.nw = CommandData.slew_veto;
      }

rvalues being an array of doubles.

This is bad because the result of the written code is not guaranteed to match the intent, so we should fix it wherever we see it.

Acceptance

Static analysis of commands.c proves a reduction of this type of violation, which is checked by many basic rulesets.

Details

There are so many of these (even in just this file) that it's likely to be a systemic issue throughout the code. Write a macro or function to perform a better floating point comparison and put it in a header or implementation file somewhere so it can be included and used elsewhere easily. You probably don't need to go as crazy as the author does in the link above, if you can make some reasoned assumptions about the majority of values we'll be comparing.

evanmayer commented 2 years ago

SciTools Understand Codecheck static analysis

Before (447b069a82101dfc165c195d66529d4665ec9297):

After (7243443c365e06655269a7c33acc1500cf316963):

I should note that upon looking at MISRA C 2012, this rule (MISRA C 2004 rule 13.3) has been revised to a more generic Directive 1.1: "Any implementation-defined behaviour on which the output of the program depends shall be documented and understood". Still a good check for us though, IMO.

This violation is reduced and we now have a tool for fixing it in other places we see it. Therefore, resolved by #14.