Not a direct answer to your questions, but after reading your code I would like to give some comments:
- You are not using "circular buffers", you are double-buffering. If you used a circular buffer, you would not need two of them.
recordingEnded
is alwaysfalse
and thus serves no purpose.- When
writing
andchange_buffer
are both true, you should transmitbuff2
, notbuff1
, as the ISR is now fillingbuff1
. - 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
. - The code that fills the buffers would be clearer, IMO, if you avoided
the repetition. E.g.
Note thatchar *buff = change_buffer ? buff1 : buff2; buff[i++] = (arpNbr << 7) | (acpPeriod >> 8); buff[i++] = acpHigh & 0xFF; buff[i++] = acpPeriod & 0xFF;
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:
- You are not using "circular buffers", you are double-buffering. If you used a circular buffer, you would not need two of them.
recordingEnded
is alwaysfalse
and thus serves no purpose.- When
writing
andchange_buffer
are both true, you should transmitbuff2
, notbuff1
, as the ISR is now fillingbuff1
. - 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
. - The code that fills the buffers would be clearer, IMO, if you avoided
the repetition. E.g.
Note thatchar *buff = change_buffer ? buff1 : buff2; buff[i++] = (arpNbr << 7) | (acpPeriod >> 8); buff[i++] = acpHigh & 0xFF; buff[i++] = acpPeriod & 0xFF;
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:
- You are not using "circular buffers", you are double-buffering. If you used a circular buffer, you would not need two of them.
recordingEnded
is alwaysfalse
and thus serves no purpose.- When
writing
andchange_buffer
are both true, you should transmitbuff2
, notbuff1
, as the ISR is now fillingbuff1
. - 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
. - The code that fills the buffers would be clearer, IMO, if you avoided
the repetition. E.g.
Note thatchar *buff = change_buffer ? buff1 : buff2; buff[i++] = (arpNbr << 7) | (acpPeriod >> 8); buff[i++] = acpHigh & 0xFF; buff[i++] = acpPeriod & 0xFF;
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:
- You are not using "circular buffers", you are double-buffering. If you used a circular buffer, you would not need two of them.
recordingEnded
is alwaysfalse
and thus serves no purpose.- When
writing
andchange_buffer
are both true, you should transmitbuff2
, notbuff1
, as the ISR is now fillingbuff1
. - 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
. - The code that fills the buffers would be clearer, IMO, if you avoided
the repetition. E.g.
Note thatchar *buff = change_buffer ? buff1 : buff2; buff[i++] = (arpNbr << 7) | (acpPeriod >> 8); buff[i++] = acpHigh & 0xFF; buff[i++] = acpPeriod & 0xFF;
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.