Fix for setChannel

[cfnz]

As mentioned in Issue #4, this fixes setChannel as it decrements variable i regardless if there is a change or not. I believe this is correct behaviour as without it, the digital pins were not getting set properly.

[RobTillaart]

@cfnz
Looked at the proposed code quickly and it looks functionally equivalent.
So the bug might be an artefact of the compiler/optimizer.
As your code is more robust / simpler, the optimizer might just generate different assembly.

Which board do you use?
Compiler version?

[cfnz]

Hi, I was running on a Wemos D1 Mini and using PlatformIO/Visual Studio Code.

But now, looking at the code I can see the issue. From my understanding, we have an array of 3 ints for the pins and the mask is being moved along to look at what each pin should be in turn. It then sets the value of the pin array, but it assumes that the index to the pin array matches the mask... but it doesn't. The index to the pin array is only moved along if that pin needed to change, so the index and the mask gets out of sync.

In my testing to find the issue I have some code, so I may as well provide it and the test output below. (I have formatted the output to line things up a bit.). The pn = the pin number and what it was set to. The Dn is the number read back from digitalRead. c is the channel number. (This output is in the middle of the stream, so c:0 came after a c:7.)

c:0   p2:0 p1:0 p0:0  D2:0  D1:0  D0:0
c:1   p2:1            D2:1  D1:0  D0:0
c:2   p2:2 p1:0       D2:1  D1:0  D0:0
c:3   p2:1            D2:1  D1:0  D0:0
c:4   p2:4 p1:0 p0:0  D2:1  D1:0  D0:0
c:5   p2:1            D2:1  D1:0  D0:0
c:6   p2:2 p1:0       D2:1  D1:0  D0:0
c:7   p2:1            D2:1  D1:0  D0:0

Here you can see that only the 3rd pin changes. Moving the i decrement out of the if statement results in the following:

c:0   p2:0 p1:0 p0:0  D2:0  D1:0  D0:0
c:1   p0:1            D2:0  D1:0  D0:1
c:2   p1:2 p0:0       D2:0  D1:1  D0:0
c:3   p0:1            D2:0  D1:1  D0:1
c:4   p2:4 p1:0 p0:0  D2:1  D1:0  D0:0
c:5   p0:1            D2:1  D1:0  D0:1
c:6   p1:2 p0:0       D2:1  D1:1  D0:0
c:7   p0:1            D2:1  D1:1  D0:1

HC4051 hc4051 = HC4051(D0, D1, D2, D3);
uint8_t  _pins[3] = { D0, D1, D2 };
uint8_t  _channel   = 0;

void loop() {
    for (int c = 0; c < 8; c++) {
        // hc4051.setChannel(c);

        Serial.print("c:");
        Serial.print(c);

        uint8_t _new = c;
        if (_new != _channel) {
            uint8_t _changed = _new ^ _channel;
            uint8_t mask = 0x04;
            uint8_t i = 2;
            Serial.print("  ");
            while (mask) {
                //  only write changed pins. //  AVR only?
                if (mask & _changed) {
                    Serial.print(" p");
                    Serial.print(i);
                    Serial.print(":");
                    Serial.print((mask & _new));
                    digitalWrite(_pins[i--], (mask & _new));
                }
                mask >>= 1;
            }
            _channel = _new;
        }

        Serial.print("  D2:");
        Serial.print(digitalRead(D2));
        Serial.print("  D1:");
        Serial.print(digitalRead(D1));
        Serial.print("  D0:");
        Serial.print(digitalRead(D0));
        Serial.println();
        Serial.println();
        delay(1000);
    }
}

(updated for syntax highlighting, adding cpp after backquotes)

[RobTillaart]

@cfnz
Oops, now I see the bug!
Good catch!

[RobTillaart]

@cfnz
Same bug exists in my HC4067 library, created an issue referring to this PR.

Will merge asap.

[RobTillaart]

@cfnz
created develop branch for a 0.3.0 version.
Added a note in the readme.md that all previous versions are obsolete.

Again thanks for reporting and the PR, really appreciated!

[cfnz]

No trouble... And thanks for your work, it is really appreciated too!