I would like feedback on the correctness and performance of this code.
My priorities are also are readability, simplicity and self-documenting code - but I'm happy with those as they are.
I still need to do some factoring of common code between the two input lines - but I'm brushing up my C and will do that.
RA2 and RA3 are the two digital input lines. My algorithm is not dissimilar to how debouncing would be done with a low pass capacitor and resistor followed by a schmitt trigger. This function is called by the ISR every 0.5ms, and the rotary switch produces transitions as short as 5-10ms.
I am curious how to deal with situations where the rotary switch is turned a bit faster than the algorithm can process. Can further information be deduced from the Gray encoding, even when a step is missed because of the low pass delay and limited sample rate? I suppose this could be taken as far as using neural networks?!
I'm mainly asking this question because I think the running time and the amount of code of the below can both be reduced by a factor of two at least. And I'd just prefer not to get out the logic analyzer and completely reinvent the wheel.
rotsw.h
#define ROTSW_AVG_SIZE 8
#define ROTSW_MAX 7
#define ROTSW_UPPER 5
#define ROTSW_LOWER 2
#define ROTSW_MIN 0
void rotsw_sample_avg_schmitt_count();
extern volatile int rotsw_count_steps;
rotsw.c
unsigned int rotsw_samples[ROTSW_AVG_SIZE];
unsigned char rotsw_write_idx = 0;
int rotsw_avg_ra2 = 0;
int rotsw_avg_ra3 = 0;
int ra2 = 0;
int ra3 = 0;
volatile int rotsw_count = 0;
void rotsw_sample_avg_schmitt_count()
{
int sample, old_sample, ra2_changed, ra3_changed, count = rotsw_count;
// Sample RA2 and RA3
old_sample = rotsw_samples[rotsw_write_idx];
rotsw_samples[rotsw_write_idx] = sample = PORTA & 0b00001100;
if (++rotsw_write_idx >= ROTSW_AVG_SIZE) rotsw_write_idx = 0;
// Moving average, turning digital sample to "analogue"
if (old_sample & 0x04) rotsw_avg_ra2--;
if (old_sample & 0x08) rotsw_avg_ra3--;
if (sample & 0x04) rotsw_avg_ra2++;
if (sample & 0x08) rotsw_avg_ra3++;
// 3. Do limit for Schmitt trigger
if (rotsw_avg_ra2 < ROTSW_MIN) rotsw_avg_ra2 = ROTSW_MIN;
else if (rotsw_avg_ra2 > ROTSW_MAX) rotsw_avg_ra2 = ROTSW_MAX;
if (rotsw_avg_ra3 < ROTSW_MIN) rotsw_avg_ra3 = ROTSW_MIN;
else if (rotsw_avg_ra3 > ROTSW_MAX) rotsw_avg_ra3 = ROTSW_MAX;
// Do Schmitt trigger
if (rotsw_avg_ra2 > ROTSW_UPPER){
ra2_changed = !ra2;
ra2 = 1;
} else if (rotsw_avg_ra2 < ROTSW_LOWER){
ra2_changed = ra2;
ra2 = 0;
}
if (rotsw_avg_ra3 > ROTSW_UPPER){
ra3_changed = !ra3;
ra3 = 1;
} else if (rotsw_avg_ra3 < ROTSW_LOWER){
ra3_changed = ra3;
ra3 = 0;
}
// Do rotsw state change count.
if (ra2_changed){
if (ra2 == ra3) count++;
else count--;
}
if (ra3_changed){
if(ra2 == ra3) count--;
else count++;
}
rotsw_count = count;
}
1 Answer 1
I don't see why bother with clipping averages. If something was less than
ROTSW_MIN
, it is also less thanROTSW_LOWER
.count
is useless, you may operate directly onrotsw_count
.
The moving averages could be computed a bit faster in assembly. The rotsw_samples
array effectively contains 8 2-bit entries, and hence could be stored in a 16-bit register, say R
. Now consider something along the lines of
rlc R ; Shift the ra2 of the "old sample" into carry
sbc avg_ra2, 0ドル ; Subtract carry
rlc R ; Same for ra3
sbc avg_ra3, 0ドル
or R, sample ; New sample is added to the "array"
rrc sample ; Shift ra3 of the "new sample" into carry
adc avg_ra3, 0ドル ; Add carry
rrc sample ; Same for ra2
adc avg_ra2, 0ドル
The exact syntax depends on the architecture and an assembler; it also assumes that the sample is already shifted right by 2.
Your target architecture may allow even faster variants.
Note that the similar tricks could be used to speed up the final rotsw_count
computation.
There is not enough context to address your worries about the switch being too speedy. In any case, the code is already quite fast. If doesn't keep up with the peripheral, the best answer is to upgrade the CPU.
Explore related questions
See similar questions with these tags.