Bug introduced in BitmaskEvent Reader

[jkbhagatio]

@glopesdev , why was this change introduced? 7b3724d#diff-7dc0f0404b6e8b4e6de8ffb857524f4426c58ad38a178c1e0a56af37408824feL196

This causes a bug when trying to read a specific event from a stream - it always returns all events, not filtered by a bitmask, if the value of the bitmask > 0.

[glopesdev]

@jkbhagatio bitmask events in digital inputs and outputs pack the state of multiple IO pins in a single integer value. For example, in Output expander AuxIn state, here are a few possible values (in least significant bit first notation, and truncated for unused middle bits, the expander uses an 8-bit word where 4 bits are state and 4 bits are event signals):

Aux0	Aux1	...	Aux0Changed	Aux1Changed	Value	Description
1	0	...	1	0	17	Rising edge on Aux0
1	1	...	0	1	35	Rising edge on Aux1 while Aux0 remains high
0	1	...	1	0	18	Falling edge on Aux0 while Aux1 remains high
1	1	...	1	0	19	Falling edge on Aux1

If we want to detect the cases where Aux0 is high, it is not enough to simply do data.event == 17 since that will miss the case where Aux0 turns high while Aux1 is high. The only way would be to check for cases where (data.event & 0x1) != 0 to get all rows where Aux0 is high regardless of rising or falling edge, or (data.event & 0x11) != 0 to get all rows where Aux0 has turned high.

This worked before because the expander on each patch happened to use only exactly one digital input, but would break the moment we start using the other input for something else.

I recommend we develop a few unit tests for the mecha repo to figure out how to adapt the current foraging schema to use the bitmasked event reader in its most general form.

[glopesdev]

@jkbhagatio but yeah, on hindsight that line should probably be changed to:

data = data[(data.event & self.value) == self.value]

To allow matching against an exact pattern of bits, rather than just accepting when any of the bits in the mask matches. I'm flagging this as a bug.

[jkbhagatio]

Ah this is still a bug: e.g. 34 & 32 = 32.

Basically if you get one event that is made up of bits of another, mutually exclusive event, I think you might be screwed?

I recommend we develop a few unit tests for the mecha repo to figure out how to adapt the current foraging schema to use the bitmasked event reader in its most general form.

^ I am definitely happy to spend time together to do this for the repo in general

[jkbhagatio]

Just thinking that for this, I guess we want make sure that we have both the ability to:

1. pull out the state for a single pin (e.g. all the times Aux0 is high, so values 17 and 35)
2. pull out cominbation states for multiple pins (e.g. specifically, rising edge on Aux1 while Aux0 remains high, 35)

Also, I guess this latter case is what's happening with BeamBreak signals? Since the hi/lo vals are 32 & 34, I guess they are somehow sharing the 32-bit pin?