Swapping modules can cause strange data updates

Flotilla
1

Hi Phil (I suppose you’ll pick this up),
Scenario is:
Flotilla Dock talking to a terminal with, for example, a light module connected. Light data gets sent as it changes as expected.
Swap the light module with, for example, a touch module on the same dock port.
Press a touch pad and it will send 1 for the touched pad, but it’ll also probably return ‘random’ values for the 2nd and 3rd pads.
Possible hint: the light module sends 3 values. If you swap in a weather module, which sends 2 large values, and then replace it with a touch, it’ll probably splatter all the pad values when you swap in a touch module and press a pad.
Press each touch pad in turn and they will then cure themselves back to sending only 1 or 0 for touched or not.
I guess you’re not clearing out the module data in the dock properly when a module is swapped for another on the same connector. Although you obviously detect and report the swap correctly with a disconnect and connect.
I had planned on ‘Hot Swappable’ functionality for my project (no, it’s not some kinky spin off of Kim Possible!)
I’m again not sure if this is a real world issue. But I think I will have to be careful what values I’m treating as a touch after a module swap. Look explicitly for 0 and 1?

Sorry for finding weird problems with flotilla, but I secretly enjoy messing about and it’s sort of my day job too. Unfortunately, I don’t seem to be able to take off my quality manager hat when I get home!

2

Thanks for the detailed bug report. It’s thorough enough that I don’t even need to reproduce. I can now clearly see the code at fault.

Each channel gets its own little place to store state from one update to the next: uint8_t data[8];

To save a few bytes, Touch stores and reads the current state of each button right from this memory:

serial_printf_P(PSTR("u %d/%s %u,%u,%u,%u\r\n"), channel->id, channel->module_type->name, *(uint8_t *)(&channel->data[3]), *(uint8_t *)(&channel->data[2]), *(uint8_t *)(&channel->data[1]), *(uint8_t *)(&channel->data[0]));

And only ever updates touch pads that have their interrupt bit set:

		for(int i = 0; i < 4; i++) {
			uint8_t bm = 1 << (i + 2); // Channels 0 and 1 aren't used

			if(value & bm){ // Interrupt for this channel
				int8_t touch_delta = i2c_read_byte(TOUCH_ADDRESS, 0x12 + i);
				int8_t touch_threshold = i2c_read_byte(TOUCH_ADDRESS, 0x32 + i);
				uint8_t last_value =  *(uint8_t *)(&channel->data[i]);
				uint8_t state = (touch_delta >= touch_threshold) ? 1 : 0;
			
				if( state != last_value ){
				
					*(uint8_t *)(&channel->data[i]) = state;
					change = true;
			
				}
			}			
		}

So if a pad isn’t updated as you point out it just sends whatever garbage it happened to be storing from the last connected module.

Needs a memset(channel->data, 0, 8); in the module detection routine just before a newly detected module is initialised.

I will roll this fix into 1.15.