Skip to main content
Arduino

Return to Answer

+ answer comment.
Source Link
Edgar Bonet
  • 45.1k
  • 4
  • 42
  • 81

Not a direct answer to your questions, but after reading your code I would like to give some comments:

  1. You are not using "circular buffers", you are double-buffering. If you used a circular buffer, you would not need two of them.
  2. recordingEnded is always false and thus serves no purpose.
  3. When writing and change_buffer are both true, you should transmit buff2, not buff1, as the ISR is now filling buff1.
  4. There are too many globals. Variables that are used in a single function should be local to the function. If you need the value to be preserved between calls, qualify the variable as static.
  5. The code that fills the buffers would be clearer, IMO, if you avoided the repetition. E.g.
    char *buff = change_buffer ? buff1 : buff2;
    buff[i++] = (arpNbr << 7) | (acpPeriod >> 8);
    buff[i++] = acpHigh & 0xFF;
    buff[i++] = acpPeriod & 0xFF;
    
    Note that buf[i++] is idiomatic C/C++.

Now, regarding your specific question, you mentioned in a comment that "there were not 3x16384 bytes between 2 ARP". This may well be a consequence of issue 3: you are not sending the bytes in the correct order, as at times (when Serial.write() runs ahead of the ISR) you may be sending old data.


Edit: to answer the comment

char *buff = change_buffer ? buff1 : buff2; won't take useless time?

Not really. Testing change_buffer is something you have to do either way. Assigning the buffer address to a local pointer is also done, under the hood, by the compiler. In the best case, it optimizes your code to something like:

register char *Z;
if (change_buffer)
 Z = buff1 + i;
else
 Z = buff2 + i;
Z[0] = (arpNbr << 7) | (acpPeriod >> 8);
Z[1] = acpHigh & 0xFF;
Z[2] = acpPeriod & 0xFF;
i += 3;

Using an explicit buff pointer gives a similar result, as the compiler will allocate this to the CPU's Z register. Actually, you may get shorter code, as the compiler may not entirely factor out the repetition.

Not a direct answer to your questions, but after reading your code I would like to give some comments:

  1. You are not using "circular buffers", you are double-buffering. If you used a circular buffer, you would not need two of them.
  2. recordingEnded is always false and thus serves no purpose.
  3. When writing and change_buffer are both true, you should transmit buff2, not buff1, as the ISR is now filling buff1.
  4. There are too many globals. Variables that are used in a single function should be local to the function. If you need the value to be preserved between calls, qualify the variable as static.
  5. The code that fills the buffers would be clearer, IMO, if you avoided the repetition. E.g.
    char *buff = change_buffer ? buff1 : buff2;
    buff[i++] = (arpNbr << 7) | (acpPeriod >> 8);
    buff[i++] = acpHigh & 0xFF;
    buff[i++] = acpPeriod & 0xFF;
    
    Note that buf[i++] is idiomatic C/C++.

Now, regarding your specific question, you mentioned in a comment that "there were not 3x16384 bytes between 2 ARP". This may well be a consequence of issue 3: you are not sending the bytes in the correct order, as at times (when Serial.write() runs ahead of the ISR) you may be sending old data.

Not a direct answer to your questions, but after reading your code I would like to give some comments:

  1. You are not using "circular buffers", you are double-buffering. If you used a circular buffer, you would not need two of them.
  2. recordingEnded is always false and thus serves no purpose.
  3. When writing and change_buffer are both true, you should transmit buff2, not buff1, as the ISR is now filling buff1.
  4. There are too many globals. Variables that are used in a single function should be local to the function. If you need the value to be preserved between calls, qualify the variable as static.
  5. The code that fills the buffers would be clearer, IMO, if you avoided the repetition. E.g.
    char *buff = change_buffer ? buff1 : buff2;
    buff[i++] = (arpNbr << 7) | (acpPeriod >> 8);
    buff[i++] = acpHigh & 0xFF;
    buff[i++] = acpPeriod & 0xFF;
    
    Note that buf[i++] is idiomatic C/C++.

Now, regarding your specific question, you mentioned in a comment that "there were not 3x16384 bytes between 2 ARP". This may well be a consequence of issue 3: you are not sending the bytes in the correct order, as at times (when Serial.write() runs ahead of the ISR) you may be sending old data.


Edit: to answer the comment

char *buff = change_buffer ? buff1 : buff2; won't take useless time?

Not really. Testing change_buffer is something you have to do either way. Assigning the buffer address to a local pointer is also done, under the hood, by the compiler. In the best case, it optimizes your code to something like:

register char *Z;
if (change_buffer)
 Z = buff1 + i;
else
 Z = buff2 + i;
Z[0] = (arpNbr << 7) | (acpPeriod >> 8);
Z[1] = acpHigh & 0xFF;
Z[2] = acpPeriod & 0xFF;
i += 3;

Using an explicit buff pointer gives a similar result, as the compiler will allocate this to the CPU's Z register. Actually, you may get shorter code, as the compiler may not entirely factor out the repetition.

Source Link
Edgar Bonet
  • 45.1k
  • 4
  • 42
  • 81

Not a direct answer to your questions, but after reading your code I would like to give some comments:

  1. You are not using "circular buffers", you are double-buffering. If you used a circular buffer, you would not need two of them.
  2. recordingEnded is always false and thus serves no purpose.
  3. When writing and change_buffer are both true, you should transmit buff2, not buff1, as the ISR is now filling buff1.
  4. There are too many globals. Variables that are used in a single function should be local to the function. If you need the value to be preserved between calls, qualify the variable as static.
  5. The code that fills the buffers would be clearer, IMO, if you avoided the repetition. E.g.
    char *buff = change_buffer ? buff1 : buff2;
    buff[i++] = (arpNbr << 7) | (acpPeriod >> 8);
    buff[i++] = acpHigh & 0xFF;
    buff[i++] = acpPeriod & 0xFF;
    
    Note that buf[i++] is idiomatic C/C++.

Now, regarding your specific question, you mentioned in a comment that "there were not 3x16384 bytes between 2 ARP". This may well be a consequence of issue 3: you are not sending the bytes in the correct order, as at times (when Serial.write() runs ahead of the ISR) you may be sending old data.

AltStyle によって変換されたページ (->オリジナル) /