Фикс: гонка _state с _e0/_e1 в uEncoderISR::tick (общий байт битовых полей)

[dkxmercury]

Ещё одна находка, теперь в новом uEncoderISR. Тут гонка между главным циклом и прерыванием, которая изредка съедает или разворачивает щелчок.

В чём дело

В tick() присвоение _state стоит после interrupts():

bool tick() {
    noInterrupts();
    uint8_t state = _buf & 0xf;
    _buf >>= 4;
    interrupts();

    _state = state ? state : State::Idle;   // <-- уже вне секции
    return state;
}

Выглядит безобидно, ведь _state больше никто не трогает. Но _state - битовое поле, и оно лежит в одном байте с _e0/_e1:

uint8_t _state : 4;
uint8_t _type : 2;
uint8_t _e0 : 1;
uint8_t _e1 : 1;

4+2+1+1 = ровно 8 бит, то есть один байт. А _e0/_e1 пишет pollRaw из tickISR, то есть из прерывания.

Запись битового поля - это read-modify-write целого байта. Вот что реально генерит avr-g++ для ATmega328 на присвоение только _state:

ld  r24,Z         ; прочитать весь байт
ori r24,lo8(15)   ; поменять свои 4 бита
st  Z,r24         ; записать весь байт обратно

Если между ld и st прилетает прерывание по CHANGE, tickISR обновляет _e0/_e1, а потом st кладёт байт обратно из r24 - со старыми ногами. Запомненное состояние ног отстаёт от реальности, и следующий переход в pollRaw считается от протухших значений: щелчок либо теряется, либо уезжает в обратную сторону. Ловится редко и выглядит как необъяснимый глюк.

Фикс

Занести присвоение внутрь секции, она и так уже есть:

noInterrupts();
uint8_t state = _buf & 0xf;
_buf >>= 4;
_state = state ? state : State::Idle;
interrupts();

Ничего не стоит по памяти и почти ничего по времени - секция удлиняется на три инструкции.

Как проверял

Раскладку не угадывал, а измерил: обнулял объект, писал одно поле, смотрел байты.

sizeof(uEncoderVirt) = 3 байта

  _state  пишет MAIN -> [0]=0x0F [1]=0x00 [2]=0x00
  _type   пишет MAIN -> [0]=0x30 [1]=0x00 [2]=0x00
  _e0     пишет ISR  -> [0]=0x40 [1]=0x00 [2]=0x00
  _e1     пишет ISR  -> [0]=0x80 [1]=0x00 [2]=0x00
  _pos    пишет ISR  -> [0]=0x00 [1]=0x0F [2]=0x00
  _rev    пишет MAIN -> [0]=0x00 [1]=0x10 [2]=0x00
  _tmr    пишет ISR  -> [0]=0x00 [1]=0x00 [2]=0xFF

_state, _e0, _e1 - все в байте 0. На ATmega328 раскладка та же, static_assert(sizeof(uEncoderVirt) == 3) проходит. Ассемблер выше - оттуда же, avr-g++ 5.4.0, -Os -mmcu=atmega328p. После правки собирается чисто и на AVR, и на x86, предупреждений нет.

Ещё два места рядом, чинить не стал

Не тащу в этот PR, чтобы не мешать всё в кучу, но раз уж копал:

1. reset() пишет _state из главного цикла - тот же байт, та же гонка. В опросном режиме неважно (прерывания нет), а в ISR-режиме ловится теоретически.
2. _rev и _pos делят байт 1 (видно в замере выше). setEncReverse() обычно зовут один раз в setup() до attachInterrupt, так что на практике не стреляет.

Если это в принципе интересно - обе пары можно развести вообще без потери памяти, просто переставив поля так, чтобы в одном байте лежало только то, что пишет главный цикл, а в другом только то, что пишет прерывание. По битам сходится в те же 3 байта. Но это уже вопрос вкуса, и лезть в раскладку без спроса не буду.

---

Видел твой ответ в uButton#3 про то, что PR не принимаешь и пишешь сам в своём стиле - твоё право, спорить не собираюсь. Если возьмёшь как есть, буду рад. Если удобнее переписать по-своему - тоже отлично, главное чтобы починилось.