2
\$\begingroup\$

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;
}
\$\endgroup\$
1
\$\begingroup\$
  • I don't see why bother with clipping averages. If something was less than ROTSW_MIN, it is also less than ROTSW_LOWER.

  • count is useless, you may operate directly on rotsw_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.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.