DEV-574 improved throw bytes performance and console prints by marknolan · Pull Request #256 · ShimmerResearch/Shimmer-Java-Android-API

marknolan · 2025-11-26T08:09:42Z

Even with checking for start 0x00/0xFF bytes on packets and with 2 byte CRC check, in bad conditions rare corrupted packets can slip through the net and appear as good packets. When this occurs, it can lead parsing to skip ahead 8.5mins as SW thinks a clock roll-over has mistakenly occurred. This PR adds a contiguous check to make sure packets are within 10 seconds of each other before parsing (number chosen arbitrarily).
When issues occurred with packets, bytes were previously thrown one at a time looking for the next 0xFF or 0x00 bytes (i.e., the start of the next packet). This was not only inefficient but leads to significant console prints for each thrown byte. This PR scans for the index of the next 0xFF/0x00 and throws bytes up to that point all in one go.

…ata_packed_passing_CRC

Copilot

Pull request overview

This PR adds protection against corrupted data packets that pass CRC validation and improves the efficiency of byte discarding when packet synchronization errors occur. The changes address two main issues: (1) rare corrupted packets causing incorrect timestamp rollover detection, and (2) inefficient byte-by-byte discarding with excessive console output.

Key Changes

Added contiguous timestamp validation to reject packets with timestamps differing by more than 10 seconds from the previous packet
Replaced byte-by-byte discarding with bulk scanning to find the next packet start marker (0x00 or 0xFF)
Enhanced debug logging to include packet length information

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 11 comments.

File	Description
ShimmerObject.java	Added CONTIGUOUS_TIMESTAMP_TICKS_LIMIT constant and contiguous timestamp check logic to filter corrupted packets; added mLastReceivedTimeStampTicks field to track last valid timestamp
ObjectCluster.java	Added successfullyParsed boolean flag to indicate when packets fail validation checks
ShimmerBluetooth.java	Refactored discardFirstBufferByte() to discardBufferBytesToNextPacket() with bulk scanning; added findOffsetOfNextZeroOrFF() helper method; improved debug logging with packet lengths; added check to skip packets marked as unsuccessfully parsed

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

ShimmerDriver/src/main/java/com/shimmerresearch/driver/ShimmerObject.java

ShimmerDriver/src/main/java/com/shimmerresearch/bluetooth/ShimmerBluetooth.java

ShimmerDriver/src/main/java/com/shimmerresearch/driver/ObjectCluster.java

ShimmerDriver/src/main/java/com/shimmerresearch/bluetooth/ShimmerBluetooth.java

ShimmerDriver/src/main/java/com/shimmerresearch/driver/ShimmerObject.java

ShimmerDriver/src/main/java/com/shimmerresearch/bluetooth/ShimmerBluetooth.java

ShimmerDriver/src/main/java/com/shimmerresearch/driver/ShimmerObject.java

…merBluetooth.java Co-authored-by: Copilot <[email protected]>

…luster.java Co-authored-by: Copilot <[email protected]>

…Object.java Co-authored-by: Copilot <[email protected]>

…merBluetooth.java Co-authored-by: Copilot <[email protected]>

marknolan · 2025-12-01T11:11:19Z

@JongChern there are two aspects to this PR

More efficient discarding of misaligned bytes and more efficient console prints related to this
Contiguous packet check

I believe 1) is an improvement that's worthy of merging once reviewed.

Point 2 can be discussed if needed. I think it is an issue but it's only particularly prominent at the moment as the FW is dropping more bytes than usual due to an as yet to be determined bug.

I can separate out these points on two different PRs if needed.

marknolan · 2025-12-01T13:30:53Z

@JongChern there are two aspects to this PR

More efficient discarding of misaligned bytes and more efficient console prints related to this

Contiguous packet check

I believe 1) is an improvement that's worthy of merging once reviewed.

Point 2 can be discussed if needed. I think it is an issue but it's only particularly prominent at the moment as the FW is dropping more bytes than usual due to an as yet to be determined bug.

I can separate out these points on two different PRs if needed.

I still believe point 1 is good. Please hold on point 2 until we resolve some issues with FW

marknolan · 2025-12-03T10:43:17Z

@JongChern this PR is ready for review.

I've removed the contiguous check for the time being. There is still a chance that the fault can occur for which the contiguous check was aiming to catch but FW fixes have made it less of an issue. I was experiencing issues with the contiguous check so it would need further development which is a lower priority now that the chances of the issue occurring have been dramatically reduced.

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

ShimmerDriver/src/main/java/com/shimmerresearch/bluetooth/ShimmerBluetooth.java:1321

@param tag "a" does not match any actual parameter of method "findOffsetOfNextZeroOrFF()".

	 * @param a byte array to search within

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

ShimmerDriver/src/main/java/com/shimmerresearch/bluetooth/ShimmerBluetooth.java

…merBluetooth.java Co-authored-by: Copilot <[email protected]>

marknolan · 2026-01-05T14:03:23Z

@JongChern the scope of this PR has reduced to the following and I think is worth merging:

More efficient discarding of misaligned bytes and more efficient console prints related to this

JongChern · 2026-01-06T06:16:40Z

@marknolan have added unit test, approving and merging now

JongChern · 2025-12-03T07:52:42Z

ShimmerDriver/src/main/java/com/shimmerresearch/driver/ShimmerObject.java

+			{
+				double shimmerTimestampTicks = (double)newPacketInt[getSignalIndex(Configuration.Shimmer3.ObjectClusterSensorName.TIMESTAMP)];
+				if(mLastReceivedTimeStampTicks!=0 
+						&& Math.abs(shimmerTimestampTicks - mLastReceivedTimeStampTicks) > CONTIGUOUS_TIMESTAMP_TICKS_LIMIT) {


hi @marknolan i believe if we use Math.abs, the test will fail for legitimate overflow, I imagine if the current timestamp is FF FF FF and the next time stamp is 00 00 00, this test will fail. i suggest removing math.abs, either that or we can perhaps consider a multiples test. e.g. if the sampling rate is 51.2Hz the next packet timestamp should be x * 640 ticks.

marknolan added 3 commits November 25, 2025 11:54

DEV-574 #time 1h better byte throwing handling and timestamp fail-safe

7c6786b

Merge remote-tracking branch 'origin/master' into DEV-574_corrupted_d…

eb801c3

…ata_packed_passing_CRC

DEV-574 #time 5m ability to disable contiguous time stamp check

9e495e6

marknolan requested a review from Copilot November 26, 2025 08:09

Copilot started reviewing on behalf of marknolan November 26, 2025 08:10 View session

Copilot finished reviewing on behalf of marknolan November 26, 2025 08:13

marknolan requested a review from JongChern November 26, 2025 08:17

Copilot AI reviewed Nov 26, 2025

View reviewed changes

marknolan and others added 8 commits November 26, 2025 08:37

DEV-574 #time 5m more efficient removal of time stamp per byte from list

61c2fca

Update ShimmerDriver/src/main/java/com/shimmerresearch/bluetooth/Shim…

101a281

…merBluetooth.java Co-authored-by: Copilot <[email protected]>

Update ShimmerDriver/src/main/java/com/shimmerresearch/bluetooth/Shim…

831e332

…merBluetooth.java Co-authored-by: Copilot <[email protected]>

Update ShimmerDriver/src/main/java/com/shimmerresearch/driver/ObjectC…

456b185

…luster.java Co-authored-by: Copilot <[email protected]>

Update ShimmerDriver/src/main/java/com/shimmerresearch/driver/Shimmer…

a21639a

…Object.java Co-authored-by: Copilot <[email protected]>

Update ShimmerDriver/src/main/java/com/shimmerresearch/bluetooth/Shim…

0d0efd6

…merBluetooth.java Co-authored-by: Copilot <[email protected]>

DEV-574 #time 1m minor update from PR

322ef02

DEV-574 #time 10m applying PR comments

834c5bc

marknolan added 2 commits December 3, 2025 10:27

DEV-574 #time 5m removed contiguous check

db28957

DEV-574 #time 10m restored some formatting and improved console print

d5b2c3d

marknolan requested a review from Copilot December 3, 2025 10:43

Copilot started reviewing on behalf of marknolan December 3, 2025 10:43 View session

Copilot finished reviewing on behalf of marknolan December 3, 2025 10:46

Copilot AI reviewed Dec 3, 2025

View reviewed changes

ShimmerDriver/src/main/java/com/shimmerresearch/bluetooth/ShimmerBluetooth.java Outdated Show resolved Hide resolved

ShimmerDriver/src/main/java/com/shimmerresearch/bluetooth/ShimmerBluetooth.java Outdated Show resolved Hide resolved

marknolan and others added 2 commits December 3, 2025 10:50

Update ShimmerDriver/src/main/java/com/shimmerresearch/bluetooth/Shim…

992069d

…merBluetooth.java Co-authored-by: Copilot <[email protected]>

Update ShimmerDriver/src/main/java/com/shimmerresearch/bluetooth/Shim…

1214fef

…merBluetooth.java Co-authored-by: Copilot <[email protected]>

marknolan changed the title ~~DEV-574 filter for corrupted data packed passing CRC and improved throw bytes performance~~ DEV-574 improved throw bytes performance and console prints Dec 4, 2025

marknolan self-assigned this Dec 16, 2025

added unit test

8693d41

JongChern approved these changes Jan 6, 2026

View reviewed changes

JongChern merged commit 7c1c0a7 into master Jan 6, 2026
1 of 3 checks passed

JongChern deleted the DEV-574_corrupted_data_packed_passing_CRC branch January 6, 2026 06:17

Conversation

marknolan commented Nov 26, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Key Changes

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

marknolan commented Dec 1, 2025

Uh oh!

marknolan commented Dec 1, 2025

Uh oh!

marknolan commented Dec 3, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

marknolan commented Jan 5, 2026

Uh oh!

JongChern commented Jan 6, 2026

Uh oh!

JongChern Dec 3, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants