Odd conditional check in output() function

In the output() function, there is this piece of code:

  if (sin_coil_A > 1024) {
    sin_coil_A = sin_coil_A - 65536;
  }

The variable sin_coil_A is declared as an int. On an ARM processor, this is 4 bytes, or the equivalent of a long on smaller Arduinos. So, if the value is larger than 1024, say 1025 as an example, you would end up with a result of -64511. I'm not sure what this is supposed to accomplish, or why you would even need to do it. Since this value is directly looked up from the sin table, and you have control over the magnitude of the table, why do you need to attempt some sort of saturation? Even if the variable was only two bytes, it would give a result that was unchanged from what you started with, i.e. if sin_coil_A = 1025, the result of 1025 - 65536 = 1025.

Note that since an int on this architecture is 4 bytes, that your sin table takes up 14400 bytes of RAM (almost half). You could declare it as 'short int' instead, which would give you two byte ints. There is sometimes a speed penalty when accessing these however, as ARMs prefer to work in their word size. Or, you could store your sin table with much higher resolution than you currently are, though there might not be much point if the DAC resolution is limited.

At any rate, I don't think this does what you intended it to do.

jcchurch13 / Mechaduino-Firmware

Odd conditional check in output() function #16