hostmot2: Fix sserial error test to use 1 << x and not 1 < x

[BsAtHome]

This is a partial fix for #4144, where it was discovered that the error check of a bit field used 1 < x instead of 1 << x.

The other issue will be cleaned up separately. See #4144 for details.

[grandixximo]

Should this be tested on hardware? @hdiethelm have anything on hand?

[hdiethelm]

Should this be tested on hardware? @hdiethelm have anything on hand?

It would make sense. However, I don't have an sserial card.

[BsAtHome]

Sure, check. But,... the error in the code is very clearly an error that would result in:

 index=0 ==> pinval & (1 < inst->remotes[inst->r_index].index) = pinval & (1 < 0) = pinval & 0
 index=1 ==> pinval & (1 < inst->remotes[inst->r_index].index) = pinval & (1 < 1) = pinval & 0
 index=2 ==> pinval & (1 < inst->remotes[inst->r_index].index) = pinval & (1 < 2) = pinval & 1
 index=3 ==> pinval & (1 < inst->remotes[inst->r_index].index) = pinval & (1 < 3) = pinval & 1
...

Therefore, you'd either not check at all (x & 0 resolves to false) and all the others would only test remote 0. And that remote may not be applicable or invalid for the instance the test was to check.

If there are installs where you get an error after this patch, then at least you know something is wrong. Currently, that chance of having a correct error is just about zero.

[grandixximo]

@pcw-mesa could you give a thumbs up?

[hdiethelm]

@BsAtHome Agreed that this looks like an error and the fix looks good. However, fixing it could surface an other bug...

Now testig is not that simple, you would have to make sure this function is called and ideally also trigger an error.

[grandixximo]

Yeah, it's more about making sure nothing else that's been hidden would surface once it runs on real hardware, it is indeed an error that never showed, the fix is correct, it's just about making sure nothing else was lurking in the dark waiting to bite :-)

[BsAtHome]

Actually, it does not change the behaviour of the code path other than emitting an error message.

The case when this hits and returns -1 is the case that initiated the #4144 report. A negative return will set the state to 100 in the statemachine, which immediately thereafter is overwritten to set another state.

So, if the statemachine is changed no longer to skip state 100, then we need a real world test that triggers the path. And this PR does not change the statemachine handling, so this just actually prints the intended message that something went wrong. And that is not a bad thing.

When the rest of #4144 is addressed, we need to assure functionality if state 100 is ever to be executed. However, the idea there was to leave it as is (and drop the error recovery state 100 completely).

[pcw-mesa]

Looks OK especially if just in master for the time being.

I think there are several weaknesses in sserial error handling but fixing obvious mistakes
is a good start.