Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

code bits are sent in the wrong order #52

Closed
rotv opened this issue May 4, 2016 · 4 comments
Closed

code bits are sent in the wrong order #52

rotv opened this issue May 4, 2016 · 4 comments

Comments

@rotv
Copy link
Contributor

rotv commented May 4, 2016

See #51 for background.

The for-loop at RCSwitch.cpp:494 sends the bits in the wrong order, it should send MSB first.

void RCSwitch::send(unsigned long code, unsigned int length) {
    for (int nRepeat = 0; nRepeat < nRepeatTransmit; nRepeat++) {
        //for (unsigned int i = 0; i < length; i++) { // incorrect order
        for (int i = length-1; i >= 0; i--) { // correct order
            if (code & (1L << i))
                this->transmit(protocol.one);
            else
                this->transmit(protocol.zero);
            }
            this->transmit(protocol.syncFactor);
        }
}
@fingolfin
Copy link
Collaborator

Note that the comment above the function explicitly states that it begins at the LSB, not at the MSB. I.e. this was intentional done. And with your proposed change in isolation, the other send() method as well as sendTriState(), would be left broken.

Since you don't mention those, I assume this means you are using send() directly?

Anyway, looking at the code, I see now that I got it wrong: I thought the old behaviour was to start at LSB, but it wasn't. It didn't help that the expected behavior of send() was not documented anywhere that I could find (not even on the Wiki).

In any case, I'll change it to work the other way around again. Thanks for reporting this!

Still: ARGH! I find it extremly weird to start at the MSB! But since that is how the code used to behave, of course it should stay like that for backwards compatibility.

@rotv
Copy link
Contributor Author

rotv commented May 4, 2016

Yeah, I noticed that. Didn't have time at the moment for a full on solution. Regarding MSB/LSB in the code I guess it's just design, but my remotes for sure wants the MSB order :) I had to look at the signals and compare to old screendumps to figure out why it wasn't working and then it was clear what was going wrong.

@fingolfin
Copy link
Collaborator

This is a bit offtopic, but: I am not sure what you mean with "your remotes want MSB order" -- first off, the order we send bits has nothing to do with the order we store them. Secondly, and more importantly, for the "encoding types" A-D we support, no actual integer values are encoded, so I don't see how one could talk about an LSB / MSB there. I guess if something was sending a value that had a natural range from 0 to N (say a rotary dial... or a temperature sensor...), then we could discuss where the LSB and MSB are in that signal, but for simple switches?!? :-)

Anyway, I hope this resolves the issue for you. If not, please reopen!

@rotv
Copy link
Contributor Author

rotv commented May 5, 2016

True. I was thinking of my previously recorded decimal codes and that the wrong bit order produced a lot of 1-0:s in the signal, which can't be expressed by the TriState letters.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants