Conversation
There was a problem hiding this comment.
Pull request overview
This PR bumps the version to 0.1.9 and addresses rare firmware glitches where Muse devices occasionally send packets with unexpected TAG bytes (e.g., 0x11 EEG4 instead of 0x12 EEG8). The fix implements channel padding with NaN values to preserve available data instead of dropping mismatched packets entirely.
Changes:
- Implemented zero-data-loss channel mismatch handling by padding mismatched packets with NaN values
- Added test data files and test suite for 2026 firmware with 0x88 battery packets and TAG variations
- Improved warning messages to include TAG byte information for easier debugging
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pyproject.toml | Version bump from 0.1.8 to 0.1.9 |
| OpenMuse/init.py | Version string update to match pyproject.toml |
| CHANGELOG.md | Detailed changelog entry documenting the fix, technical findings, and test additions |
| OpenMuse/stream.py | New _decode_with_channel_padding function to handle channel mismatches; updated _on_data to use padding logic; improved comments in _queue_samples |
| tests/test_decode.py | New TestNewFirmware test class with tests for 0x88 battery packets, 8-channel EEG, 16-channel OPTICS, and anomalous 0x11 packet detection |
| tests/test_data/data_new_firmware_anomalous.txt | Test data file with mixed 0x11 and 0x12 EEG packets for validation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| for pkt in pkt_list: | ||
| pkt_channels = pkt.get("n_channels") | ||
| if pkt_channels == expected_channels: | ||
| matching_pkts.append(pkt) | ||
| else: | ||
| mismatched_pkts.append((pkt, pkt_channels)) |
There was a problem hiding this comment.
Potential issue: If a packet doesn't have the "n_channels" key, pkt.get("n_channels") will return None. This None value is then appended to mismatched_pkts as actual_channels, which could cause a TypeError on line 335 when trying to compute 1 : actual_channels + 1 (since None + 1 is invalid). Consider adding a check to skip or handle packets where n_channels is None.
| (n_samples, 1 + expected_channels), np.nan, dtype=np.float64 | ||
| ) | ||
| padded[:, 0] = mismatch_array[:, 0] # Copy timestamp | ||
| padded[:, 1 : actual_channels + 1] = mismatch_array[:, 1:] # Copy channels |
There was a problem hiding this comment.
Potential issue: When actual_channels is greater than expected_channels, line 335 attempts to copy more data than the padded array can hold. The slice padded[:, 1 : actual_channels + 1] could be larger than mismatch_array[:, 1:] when actual_channels > expected_channels, leading to an IndexError or ValueError. Consider adding logic to handle the case where actual_channels exceeds expected_channels by only copying the first expected_channels worth of data, or by explicitly checking this condition.
| padded[:, 1 : actual_channels + 1] = mismatch_array[:, 1:] # Copy channels | |
| # Copy up to the minimum of actual, expected, and available channels | |
| channels_to_copy = min( | |
| actual_channels, | |
| expected_channels, | |
| max(0, mismatch_array.shape[1] - 1), | |
| ) | |
| if channels_to_copy > 0: | |
| padded[:, 1 : 1 + channels_to_copy] = mismatch_array[ | |
| :, 1 : 1 + channels_to_copy | |
| ] |
| def _decode_with_channel_padding( | ||
| sensor_type: str, | ||
| pkt_list: List[dict], | ||
| stream: SensorStream, | ||
| verbose: bool = True, | ||
| ) -> Tuple[np.ndarray, SensorStream]: | ||
| """ | ||
| Decode packets and handle channel count mismatches by padding with NaN. | ||
|
|
||
| This handles rare cases where Muse devices send packets with different TAG bytes | ||
| than expected (e.g., 0x11 EEG4 vs 0x12 EEG8, or 0x34/0x35/0x36 for OPTICS). | ||
| Instead of dropping mismatched packets, we preserve the available channels and | ||
| pad the missing channels with NaN. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| sensor_type : str | ||
| Sensor type identifier (EEG, ACCGYRO, OPTICS, BATTERY) | ||
| pkt_list : List[dict] | ||
| List of decoded packet dictionaries from parse_message() | ||
| stream : SensorStream | ||
| The sensor stream object containing outlet and timestamp state | ||
| verbose : bool | ||
| Whether to print warnings for mismatched packets | ||
|
|
||
| Returns | ||
| ------- | ||
| Tuple[np.ndarray, SensorStream] | ||
| Decoded data array (timestamps + channels) and updated stream object | ||
| """ | ||
| expected_channels = stream.outlet.n_channels | ||
|
|
||
| # Separate packets by channel count | ||
| matching_pkts = [] | ||
| mismatched_pkts = [] # (pkt, actual_channels) | ||
|
|
||
| for pkt in pkt_list: | ||
| pkt_channels = pkt.get("n_channels") | ||
| if pkt_channels == expected_channels: | ||
| matching_pkts.append(pkt) | ||
| else: | ||
| mismatched_pkts.append((pkt, pkt_channels)) | ||
|
|
||
| result_array = np.empty((0, 1 + expected_channels)) | ||
|
|
||
| # Process matching packets normally | ||
| if matching_pkts: | ||
| current_state = ( | ||
| stream.base_time, | ||
| stream.wrap_offset, | ||
| stream.last_abs_tick, | ||
| stream.sample_counter, | ||
| ) | ||
| array, base_time, wrap_offset, last_abs_tick, sample_counter = make_timestamps( | ||
| matching_pkts, *current_state | ||
| ) | ||
| result_array = array | ||
| stream.base_time = base_time | ||
| stream.wrap_offset = wrap_offset | ||
| stream.last_abs_tick = last_abs_tick | ||
| stream.sample_counter = sample_counter | ||
|
|
||
| # Process mismatched packets: pad with NaN to preserve available data | ||
| for pkt, actual_channels in mismatched_pkts: | ||
| if verbose: | ||
| tag_hex = hex(pkt.get("tag_byte", 0)) | ||
| print( | ||
| f"[{sensor_type}] Padding packet with {actual_channels} channels " | ||
| f"(tag={tag_hex}) to {expected_channels} channels (filling with NaN)" | ||
| ) | ||
|
|
||
| current_state = ( | ||
| stream.base_time, | ||
| stream.wrap_offset, | ||
| stream.last_abs_tick, | ||
| stream.sample_counter, | ||
| ) | ||
| mismatch_array, base_time, wrap_offset, last_abs_tick, sample_counter = ( | ||
| make_timestamps([pkt], *current_state) | ||
| ) | ||
|
|
||
| if mismatch_array.size > 0: | ||
| n_samples = mismatch_array.shape[0] | ||
| # Create padded array: timestamp + expected_channels | ||
| padded = np.full( | ||
| (n_samples, 1 + expected_channels), np.nan, dtype=np.float64 | ||
| ) | ||
| padded[:, 0] = mismatch_array[:, 0] # Copy timestamp | ||
| padded[:, 1 : actual_channels + 1] = mismatch_array[:, 1:] # Copy channels | ||
|
|
||
| # Merge with existing data | ||
| if result_array.size > 0: | ||
| result_array = np.vstack([result_array, padded]) | ||
| else: | ||
| result_array = padded | ||
|
|
||
| stream.base_time = base_time | ||
| stream.wrap_offset = wrap_offset | ||
| stream.last_abs_tick = last_abs_tick | ||
| stream.sample_counter = sample_counter | ||
|
|
||
| return result_array, stream |
There was a problem hiding this comment.
The new _decode_with_channel_padding function in stream.py lacks test coverage. While tests exist for detecting anomalous packets in test_decode.py, there are no tests that verify the channel padding behavior works correctly during streaming. Consider adding tests that verify: 1) packets with fewer channels are correctly padded with NaN, 2) the stream state is properly updated after processing mismatched packets, and 3) the function handles edge cases like None channel counts or packets with more channels than expected.
1 participant
No description provided.