Skip to content

Commit e3e78e1

Browse files
romtsnclaudegetsentry-bot
authored
fix(feedback): Improve shake detection sensitivity (#5366)
* fix(feedback): Improve shake detection sensitivity Replace the threshold-counting approach (2.7g, 2 spikes in 1.5s) with a rolling sample window based on Square's Seismic library. A shake is now detected when >75% of accelerometer readings in a 0.5s window exceed 13 m/s² (~1.33g), which works reliably on budget devices with less sensitive accelerometers. Fixes GH-5331 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * docs: Add license attribution for Square's Seismic library Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * docs: Add third-party code attribution guidelines to AGENTS.md Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Format code * docs(changelog): Add shake detection fix entry Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(feedback): Clear message field when form is re-shown via shake Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(feedback): Synchronize SampleQueue access across threads stop() runs on the main thread while onSensorChanged() runs on the background HandlerThread. Without synchronization, concurrent access to the linked list and object pool can corrupt next-pointers and cause clear() to loop forever. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * ref(feedback): Replace synchronized block with handler.post for queue clear Post queue.clear() to the HandlerThread instead of synchronizing every sensor event. All queue access now stays single-threaded with zero lock contention. quitSafely() drains pending messages before exiting. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * dont use method ref * ref(feedback): Remove queue.clear() from stop(), rely on timestamp purge Sensor events are delivered via fd callbacks, not Handler messages, so posting clear() to the HandlerThread doesn't guarantee ordering with new events after re-registration. The SampleQueue already purges stale samples by timestamp in add(), so explicit clearing on stop is unnecessary. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * ref(feedback): Restore handler.post(clear) in stop() Both fd callbacks and posted Messages are serialized by the Looper, so there is no concurrent access risk. Explicit clear is cleaner than relying on timestamp purge alone. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(feedback): Require minimum sample count before triggering shake The 75% bit-shift formula degrades at low sample counts (e.g. 33% at 3 samples). With SENSOR_DELAY_NORMAL (~5Hz) the queue may hold only 3 samples in 0.5s. Adding a MIN_QUEUE_SIZE guard ensures the threshold stays accurate and prevents false triggers from walking. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Sentry Github Bot <bot+github-bot@sentry.io>
1 parent d25ef95 commit e3e78e1

6 files changed

Lines changed: 239 additions & 56 deletions

File tree

AGENTS.md

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,17 @@ The repository is organized into multiple modules:
144144
4. New features must be **opt-in by default** - extend `SentryOptions` or similar Option classes with getters/setters
145145
5. Consider backwards compatibility
146146

147+
### Third-Party Code Attribution
148+
When adapting code from third-party libraries:
149+
1. Add a license header at the top of the adapted file (before the `package` statement):
150+
```java
151+
// Adapted from <Library Name>.
152+
// Copyright <year> <copyright holder>.
153+
// Licensed under the <License Name>.
154+
// <source URL>
155+
```
156+
2. Add a full attribution entry to `THIRD_PARTY_NOTICES.md` following the existing format (Source, License, Copyright, Scope, full license text)
157+
147158
### Getting PR Information
148159

149160
Use `gh pr view` to get PR details from the current branch. This is needed when adding changelog entries, which require the PR number.

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@
2323
### Fixes
2424

2525
- Fix soft input keyboard not being shown on the Feedback form ([#5359](https://github.com/getsentry/sentry-java/pull/5359))
26+
- Fix shake-to-report not triggering on some devices due to high acceleration threshold ([#5366](https://github.com/getsentry/sentry-java/pull/5366))
27+
- Fix feedback form retaining previous message when shown again via shake ([#5366](https://github.com/getsentry/sentry-java/pull/5366))
2628

2729
### Dependencies
2830

THIRD_PARTY_NOTICES.md

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,34 @@ limitations under the License.
118118

119119
---
120120

121+
## Square — Seismic (Apache 2.0)
122+
123+
**Source:** https://github.com/square/seismic<br>
124+
**License:** Apache License 2.0<br>
125+
**Copyright:** Copyright 2010 Square, Inc.
126+
127+
### Scope
128+
129+
The Sentry Java SDK includes an adapted version of Square's Seismic shake detection algorithm. The rolling sample window approach and `SampleQueue`/`SamplePool` data structures in `io.sentry.android.core.SentryShakeDetector` are based on Seismic's `ShakeDetector`.
130+
131+
```
132+
Copyright 2010 Square, Inc.
133+
134+
Licensed under the Apache License, Version 2.0 (the "License");
135+
you may not use this file except in compliance with the License.
136+
You may obtain a copy of the License at
137+
138+
http://www.apache.org/licenses/LICENSE-2.0
139+
140+
Unless required by applicable law or agreed to in writing, software
141+
distributed under the License is distributed on an "AS IS" BASIS,
142+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
143+
See the License for the specific language governing permissions and
144+
limitations under the License.
145+
```
146+
147+
---
148+
121149
## Square — Curtains (Apache 2.0)
122150

123151
**Source:** https://github.com/square/curtains (v1.2.5)<br>

sentry-android-core/src/main/java/io/sentry/android/core/SentryShakeDetector.java

Lines changed: 121 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
// Adapted from Square's Seismic library.
2+
// Copyright 2010 Square, Inc.
3+
// Licensed under the Apache License, Version 2.0.
4+
// https://github.com/square/seismic
15
package io.sentry.android.core;
26

37
import android.content.Context;
@@ -7,10 +11,8 @@
711
import android.hardware.SensorManager;
812
import android.os.Handler;
913
import android.os.HandlerThread;
10-
import android.os.SystemClock;
1114
import io.sentry.ILogger;
1215
import io.sentry.SentryLevel;
13-
import java.util.concurrent.atomic.AtomicLong;
1416
import org.jetbrains.annotations.ApiStatus;
1517
import org.jetbrains.annotations.NotNull;
1618
import org.jetbrains.annotations.Nullable;
@@ -21,30 +23,25 @@
2123
* <p>The accelerometer sensor (TYPE_ACCELEROMETER) does NOT require any special permissions on
2224
* Android. The BODY_SENSORS permission is only needed for heart rate and similar body sensors.
2325
*
24-
* <p>Requires at least {@link #SHAKE_COUNT_THRESHOLD} accelerometer readings above {@link
25-
* #SHAKE_THRESHOLD_GRAVITY} within {@link #SHAKE_WINDOW_MS} to trigger a shake event.
26+
* <p>Uses a rolling sample window: if more than 75% of accelerometer readings in the past 0.5s
27+
* exceed {@link #ACCELERATION_THRESHOLD}, a shake is detected. Based on Square's Seismic library.
2628
*
2729
* <p>Sensor events are delivered on a background {@link HandlerThread} to avoid polluting the main
2830
* thread.
2931
*/
3032
@ApiStatus.Internal
3133
public final class SentryShakeDetector implements SensorEventListener {
3234

33-
private static final float SHAKE_THRESHOLD_GRAVITY = 2.7f;
34-
private static final int SHAKE_WINDOW_MS = 1500;
35-
private static final int SHAKE_COUNT_THRESHOLD = 2;
36-
private static final int SHAKE_COOLDOWN_MS = 1000;
35+
static final int ACCELERATION_THRESHOLD = 13;
3736

3837
private @Nullable SensorManager sensorManager;
3938
private @Nullable Sensor accelerometer;
4039
private @Nullable HandlerThread handlerThread;
4140
private @Nullable Handler handler;
42-
private final @NotNull AtomicLong lastShakeTimestamp = new AtomicLong(0);
4341
private volatile @Nullable Listener listener;
4442
private @NotNull ILogger logger;
4543

46-
private int shakeCount = 0;
47-
private long firstShakeTimestamp = 0;
44+
private final @NotNull SampleQueue queue = new SampleQueue();
4845

4946
public interface Listener {
5047
void onShake();
@@ -94,17 +91,24 @@ public void start(final @NotNull Context context, final @NotNull Listener shakeL
9491

9592
public void stop() {
9693
listener = null;
97-
shakeCount = 0;
98-
firstShakeTimestamp = 0;
9994
if (sensorManager != null) {
10095
sensorManager.unregisterListener(this);
10196
}
97+
final @Nullable Handler h = handler;
98+
if (h != null) {
99+
h.post(
100+
() -> {
101+
//noinspection Convert2MethodRef
102+
queue.clear();
103+
});
104+
}
102105
}
103106

104107
/** Stops detection and releases the background thread. */
105108
public void close() {
106109
stop();
107110
if (handlerThread != null) {
111+
// quitSafely drains pending messages (including the clear posted by stop) before exiting
108112
handlerThread.quitSafely();
109113
handlerThread = null;
110114
handler = null;
@@ -116,32 +120,17 @@ public void onSensorChanged(final @NotNull SensorEvent event) {
116120
if (event.sensor.getType() != Sensor.TYPE_ACCELEROMETER) {
117121
return;
118122
}
119-
float gX = event.values[0] / SensorManager.GRAVITY_EARTH;
120-
float gY = event.values[1] / SensorManager.GRAVITY_EARTH;
121-
float gZ = event.values[2] / SensorManager.GRAVITY_EARTH;
122-
double gForceSquared = gX * gX + gY * gY + gZ * gZ;
123-
if (gForceSquared > SHAKE_THRESHOLD_GRAVITY * SHAKE_THRESHOLD_GRAVITY) {
124-
long now = SystemClock.elapsedRealtime();
125-
126-
// Reset counter if outside the detection window
127-
if (now - firstShakeTimestamp > SHAKE_WINDOW_MS) {
128-
shakeCount = 0;
129-
firstShakeTimestamp = now;
130-
}
131-
132-
shakeCount++;
133-
134-
if (shakeCount >= SHAKE_COUNT_THRESHOLD) {
135-
// Enforce cooldown so we don't fire repeatedly
136-
long lastShake = lastShakeTimestamp.get();
137-
if (now - lastShake > SHAKE_COOLDOWN_MS) {
138-
lastShakeTimestamp.set(now);
139-
shakeCount = 0;
140-
final @Nullable Listener currentListener = listener;
141-
if (currentListener != null) {
142-
currentListener.onShake();
143-
}
144-
}
123+
final float ax = event.values[0];
124+
final float ay = event.values[1];
125+
final float az = event.values[2];
126+
final boolean accelerating = Math.sqrt(ax * ax + ay * ay + az * az) > ACCELERATION_THRESHOLD;
127+
128+
queue.add(event.timestamp, accelerating);
129+
if (queue.isShaking()) {
130+
queue.clear();
131+
final @Nullable Listener currentListener = listener;
132+
if (currentListener != null) {
133+
currentListener.onShake();
145134
}
146135
}
147136
}
@@ -150,4 +139,97 @@ public void onSensorChanged(final @NotNull SensorEvent event) {
150139
public void onAccuracyChanged(final @NotNull Sensor sensor, final int accuracy) {
151140
// Not needed for shake detection.
152141
}
142+
143+
static class SampleQueue {
144+
private static final long MAX_WINDOW_SIZE_NS = 500_000_000L; // 0.5s
145+
private static final long MIN_WINDOW_SIZE_NS = MAX_WINDOW_SIZE_NS >> 1; // 0.25s
146+
private static final int MIN_QUEUE_SIZE = 4;
147+
148+
private final @NotNull SamplePool pool = new SamplePool();
149+
private @Nullable Sample oldest;
150+
private @Nullable Sample newest;
151+
private int sampleCount;
152+
private int acceleratingCount;
153+
154+
void add(final long timestamp, final boolean accelerating) {
155+
purge(timestamp - MAX_WINDOW_SIZE_NS);
156+
157+
final @NotNull Sample added = pool.acquire();
158+
added.timestamp = timestamp;
159+
added.accelerating = accelerating;
160+
added.next = null;
161+
if (newest != null) {
162+
newest.next = added;
163+
}
164+
newest = added;
165+
if (oldest == null) {
166+
oldest = added;
167+
}
168+
169+
sampleCount++;
170+
if (accelerating) {
171+
acceleratingCount++;
172+
}
173+
}
174+
175+
void clear() {
176+
while (oldest != null) {
177+
final @NotNull Sample removed = oldest;
178+
oldest = removed.next;
179+
pool.release(removed);
180+
}
181+
newest = null;
182+
sampleCount = 0;
183+
acceleratingCount = 0;
184+
}
185+
186+
private void purge(final long cutoff) {
187+
while (sampleCount >= MIN_QUEUE_SIZE && oldest != null && cutoff - oldest.timestamp > 0) {
188+
final @NotNull Sample removed = oldest;
189+
if (removed.accelerating) {
190+
acceleratingCount--;
191+
}
192+
sampleCount--;
193+
oldest = removed.next;
194+
if (oldest == null) {
195+
newest = null;
196+
}
197+
pool.release(removed);
198+
}
199+
}
200+
201+
boolean isShaking() {
202+
return newest != null
203+
&& oldest != null
204+
&& sampleCount >= MIN_QUEUE_SIZE
205+
&& newest.timestamp - oldest.timestamp >= MIN_WINDOW_SIZE_NS
206+
&& acceleratingCount >= (sampleCount >> 1) + (sampleCount >> 2);
207+
}
208+
}
209+
210+
static class Sample {
211+
long timestamp;
212+
boolean accelerating;
213+
@Nullable Sample next;
214+
}
215+
216+
static class SamplePool {
217+
private @Nullable Sample head;
218+
219+
@NotNull
220+
Sample acquire() {
221+
Sample acquired = head;
222+
if (acquired == null) {
223+
acquired = new Sample();
224+
} else {
225+
head = acquired.next;
226+
}
227+
return acquired;
228+
}
229+
230+
void release(final @NotNull Sample sample) {
231+
sample.next = head;
232+
head = sample;
233+
}
234+
}
153235
}

sentry-android-core/src/main/java/io/sentry/android/core/SentryUserFeedbackForm.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -324,6 +324,12 @@ public void setOnDismissListener(final @Nullable OnDismissListener listener) {
324324
@Override
325325
protected void onStart() {
326326
super.onStart();
327+
// Clear the message field so subsequent show() calls start with a fresh form
328+
final @NotNull EditText edtMessage =
329+
findViewById(R.id.sentry_dialog_user_feedback_edt_description);
330+
edtMessage.getText().clear();
331+
edtMessage.setError(null);
332+
327333
final @NotNull SentryOptions options = Sentry.getCurrentScopes().getOptions();
328334
final @NotNull SentryFeedbackOptions feedbackOptions = options.getFeedbackOptions();
329335
final @Nullable Runnable onFormOpen = feedbackOptions.getOnFormOpen();

0 commit comments

Comments
 (0)