Skip to content

Commit 177bb48

Browse files
antonisclaude
antonis
and
claude
committed
fix(feedback): improve shake detection robustness and add tests
- Add volatile/AtomicLong for thread-safe cross-thread field access - Use SystemClock.elapsedRealtime() instead of System.currentTimeMillis() - Use SENSOR_DELAY_NORMAL for better battery efficiency - Add multi-shake counting (2+ threshold crossings within 1.5s window) - Handle deferred init for already-resumed activities - Wrap showDialog() in try-catch to prevent app crashes - Improve activity transition handling in onActivityPaused - Mark SentryShakeDetector as @ApiStatus.Internal - Add unit tests for SentryShakeDetector and ShakeDetectionIntegration Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent c04bebe commit 177bb48

File tree

7 files changed

+352
-13
lines changed

7 files changed

+352
-13
lines changed

‎CHANGELOG.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,9 @@
44

55
### Features
66

7+
- Show feedback form on device shake ([#5150](https://github.com/getsentry/sentry-java/pull/5150))
8+
- Enable via `options.getFeedbackOptions().setUseShakeGesture(true)`
9+
- Uses the device's accelerometer — no special permissions required
710
- Create `sentry-opentelemetry-otlp` and `sentry-opentelemetry-otlp-spring` modules for combining OpenTelemetry SDK OTLP export with Sentry SDK ([#5100](https://github.com/getsentry/sentry-java/pull/5100))
811
- OpenTelemetry is configured to send spans to Sentry directly using an OTLP endpoint.
912
- Sentry only uses trace and span ID from OpenTelemetry (via `OpenTelemetryOtlpEventProcessor`) but will not send spans through OpenTelemetry nor use OpenTelemetry `Context` for `Scopes` propagation.

‎sentry-android-core/api/sentry-android-core.api‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -457,6 +457,18 @@ public final class io/sentry/android/core/SentryScreenshotOptions : io/sentry/Se
457457
public fun trackCustomMasking ()V
458458
}
459459

460+
public final class io/sentry/android/core/SentryShakeDetector : android/hardware/SensorEventListener {
461+
public fun <init> (Lio/sentry/ILogger;)V
462+
public fun onAccuracyChanged (Landroid/hardware/Sensor;I)V
463+
public fun onSensorChanged (Landroid/hardware/SensorEvent;)V
464+
public fun start (Landroid/content/Context;Lio/sentry/android/core/SentryShakeDetector$Listener;)V
465+
public fun stop ()V
466+
}
467+
468+
public abstract interface class io/sentry/android/core/SentryShakeDetector$Listener {
469+
public abstract fun onShake ()V
470+
}
471+
460472
public class io/sentry/android/core/SentryUserFeedbackButton : android/widget/Button {
461473
public fun <init> (Landroid/content/Context;)V
462474
public fun <init> (Landroid/content/Context;Landroid/util/AttributeSet;)V
@@ -485,6 +497,19 @@ public abstract interface class io/sentry/android/core/SentryUserFeedbackDialog$
485497
public abstract fun configure (Landroid/content/Context;Lio/sentry/SentryFeedbackOptions;)V
486498
}
487499

500+
public final class io/sentry/android/core/ShakeDetectionIntegration : android/app/Application$ActivityLifecycleCallbacks, io/sentry/Integration, java/io/Closeable {
501+
public fun <init> (Landroid/app/Application;)V
502+
public fun close ()V
503+
public fun onActivityCreated (Landroid/app/Activity;Landroid/os/Bundle;)V
504+
public fun onActivityDestroyed (Landroid/app/Activity;)V
505+
public fun onActivityPaused (Landroid/app/Activity;)V
506+
public fun onActivityResumed (Landroid/app/Activity;)V
507+
public fun onActivitySaveInstanceState (Landroid/app/Activity;Landroid/os/Bundle;)V
508+
public fun onActivityStarted (Landroid/app/Activity;)V
509+
public fun onActivityStopped (Landroid/app/Activity;)V
510+
public fun register (Lio/sentry/IScopes;Lio/sentry/SentryOptions;)V
511+
}
512+
488513
public class io/sentry/android/core/SpanFrameMetricsCollector : io/sentry/IPerformanceContinuousCollector, io/sentry/android/core/internal/util/SentryFrameMetricsCollector$FrameMetricsCollectorListener {
489514
protected final field lock Lio/sentry/util/AutoClosableReentrantLock;
490515
public fun <init> (Lio/sentry/android/core/SentryAndroidOptions;Lio/sentry/android/core/internal/util/SentryFrameMetricsCollector;)V

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

Lines changed: 35 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,11 @@
55
import android.hardware.SensorEvent;
66
import android.hardware.SensorEventListener;
77
import android.hardware.SensorManager;
8+
import android.os.SystemClock;
89
import io.sentry.ILogger;
910
import io.sentry.SentryLevel;
11+
import java.util.concurrent.atomic.AtomicLong;
12+
import org.jetbrains.annotations.ApiStatus;
1013
import org.jetbrains.annotations.NotNull;
1114
import org.jetbrains.annotations.Nullable;
1215

@@ -15,17 +18,26 @@
1518
*
1619
* <p>The accelerometer sensor (TYPE_ACCELEROMETER) does NOT require any special permissions on
1720
* Android. The BODY_SENSORS permission is only needed for heart rate and similar body sensors.
21+
*
22+
* <p>Requires at least {@link #SHAKE_COUNT_THRESHOLD} accelerometer readings above {@link
23+
* #SHAKE_THRESHOLD_GRAVITY} within {@link #SHAKE_WINDOW_MS} to trigger a shake event.
1824
*/
25+
@ApiStatus.Internal
1926
public final class SentryShakeDetector implements SensorEventListener {
2027

2128
private static final float SHAKE_THRESHOLD_GRAVITY = 2.7f;
29+
private static final int SHAKE_WINDOW_MS = 1500;
30+
private static final int SHAKE_COUNT_THRESHOLD = 2;
2231
private static final int SHAKE_COOLDOWN_MS = 1000;
2332

2433
private @Nullable SensorManager sensorManager;
25-
private long lastShakeTimestamp = 0;
26-
private @Nullable Listener listener;
34+
private final @NotNull AtomicLong lastShakeTimestamp = new AtomicLong(0);
35+
private volatile @Nullable Listener listener;
2736
private final @NotNull ILogger logger;
2837

38+
private int shakeCount = 0;
39+
private long firstShakeTimestamp = 0;
40+
2941
public interface Listener {
3042
void onShake();
3143
}
@@ -47,7 +59,7 @@ public void start(final @NotNull Context context, final @NotNull Listener shakeL
4759
SentryLevel.WARNING, "Accelerometer sensor not available. Shake detection disabled.");
4860
return;
4961
}
50-
sensorManager.registerListener(this, accelerometer, SensorManager.SENSOR_DELAY_UI);
62+
sensorManager.registerListener(this, accelerometer, SensorManager.SENSOR_DELAY_NORMAL);
5163
}
5264

5365
public void stop() {
@@ -68,11 +80,26 @@ public void onSensorChanged(final @NotNull SensorEvent event) {
6880
float gZ = event.values[2] / SensorManager.GRAVITY_EARTH;
6981
double gForce = Math.sqrt(gX * gX + gY * gY + gZ * gZ);
7082
if (gForce > SHAKE_THRESHOLD_GRAVITY) {
71-
long now = System.currentTimeMillis();
72-
if (now - lastShakeTimestamp > SHAKE_COOLDOWN_MS) {
73-
lastShakeTimestamp = now;
74-
if (listener != null) {
75-
listener.onShake();
83+
long now = SystemClock.elapsedRealtime();
84+
85+
// Reset counter if outside the detection window
86+
if (now - firstShakeTimestamp > SHAKE_WINDOW_MS) {
87+
shakeCount = 0;
88+
firstShakeTimestamp = now;
89+
}
90+
91+
shakeCount++;
92+
93+
if (shakeCount >= SHAKE_COUNT_THRESHOLD) {
94+
// Enforce cooldown so we don't fire repeatedly
95+
long lastShake = lastShakeTimestamp.get();
96+
if (now - lastShake > SHAKE_COOLDOWN_MS) {
97+
lastShakeTimestamp.set(now);
98+
shakeCount = 0;
99+
final @Nullable Listener currentListener = listener;
100+
if (currentListener != null) {
101+
currentListener.onShake();
102+
}
76103
}
77104
}
78105
}

‎sentry-android-core/src/main/java/io/sentry/android/core/ShakeDetectionIntegration.java‎

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ public final class ShakeDetectionIntegration
2525
private final @NotNull Application application;
2626
private @Nullable SentryShakeDetector shakeDetector;
2727
private @Nullable SentryAndroidOptions options;
28-
private @Nullable Activity currentActivity;
28+
private volatile @Nullable Activity currentActivity;
2929

3030
public ShakeDetectionIntegration(final @NotNull Application application) {
3131
this.application = Objects.requireNonNull(application, "Application is required");
@@ -42,6 +42,13 @@ public void register(final @NotNull IScopes scopes, final @NotNull SentryOptions
4242
addIntegrationToSdkVersion("ShakeDetection");
4343
application.registerActivityLifecycleCallbacks(this);
4444
options.getLogger().log(SentryLevel.DEBUG, "ShakeDetectionIntegration installed.");
45+
46+
// In case of a deferred init, hook into any already-resumed activity
47+
final @Nullable Activity activity = CurrentActivityHolder.getInstance().getActivity();
48+
if (activity != null) {
49+
currentActivity = activity;
50+
startShakeDetection(activity);
51+
}
4552
}
4653

4754
@Override
@@ -58,8 +65,13 @@ public void onActivityResumed(final @NotNull Activity activity) {
5865

5966
@Override
6067
public void onActivityPaused(final @NotNull Activity activity) {
61-
stopShakeDetection();
62-
currentActivity = null;
68+
// Only stop if this is the activity we're tracking. When transitioning between
69+
// activities, B.onResume may fire before A.onPause — stopping unconditionally
70+
// would kill shake detection for the new activity.
71+
if (activity == currentActivity) {
72+
stopShakeDetection();
73+
currentActivity = null;
74+
}
6375
}
6476

6577
@Override
@@ -80,17 +92,27 @@ public void onActivitySaveInstanceState(
8092
public void onActivityDestroyed(final @NotNull Activity activity) {}
8193

8294
private void startShakeDetection(final @NotNull Activity activity) {
83-
if (shakeDetector != null || options == null) {
95+
if (options == null) {
8496
return;
8597
}
98+
// Stop any existing detector (e.g. when transitioning between activities)
99+
stopShakeDetection();
86100
shakeDetector = new SentryShakeDetector(options.getLogger());
87101
shakeDetector.start(
88102
activity,
89103
() -> {
90104
final Activity active = currentActivity;
91105
if (active != null && options != null) {
92106
active.runOnUiThread(
93-
() -> options.getFeedbackOptions().getDialogHandler().showDialog(null, null));
107+
() -> {
108+
try {
109+
options.getFeedbackOptions().getDialogHandler().showDialog(null, null);
110+
} catch (Throwable e) {
111+
options
112+
.getLogger()
113+
.log(SentryLevel.ERROR, "Failed to show feedback dialog on shake.", e);
114+
}
115+
});
94116
}
95117
});
96118
}

‎sentry-android-core/src/test/java/io/sentry/android/core/SentryShakeDetectorTest.kt‎

Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,154 @@
1+
package io.sentry.android.core
2+
3+
import android.content.Context
4+
import android.hardware.Sensor
5+
import android.hardware.SensorEvent
6+
import android.hardware.SensorManager
7+
import android.os.SystemClock
8+
import androidx.test.ext.junit.runners.AndroidJUnit4
9+
import io.sentry.ILogger
10+
import kotlin.test.Test
11+
import org.junit.runner.RunWith
12+
import org.mockito.kotlin.any
13+
import org.mockito.kotlin.eq
14+
import org.mockito.kotlin.mock
15+
import org.mockito.kotlin.never
16+
import org.mockito.kotlin.verify
17+
import org.mockito.kotlin.whenever
18+
19+
@RunWith(AndroidJUnit4::class)
20+
class SentryShakeDetectorTest {
21+
22+
private class Fixture {
23+
val logger = mock<ILogger>()
24+
val context = mock<Context>()
25+
val sensorManager = mock<SensorManager>()
26+
val accelerometer = mock<Sensor>()
27+
val listener = mock<SentryShakeDetector.Listener>()
28+
29+
init {
30+
whenever(context.getSystemService(Context.SENSOR_SERVICE)).thenReturn(sensorManager)
31+
whenever(sensorManager.getDefaultSensor(Sensor.TYPE_ACCELEROMETER)).thenReturn(accelerometer)
32+
}
33+
34+
fun getSut(): SentryShakeDetector {
35+
return SentryShakeDetector(logger)
36+
}
37+
}
38+
39+
private val fixture = Fixture()
40+
41+
@Test
42+
fun `registers sensor listener on start`() {
43+
val sut = fixture.getSut()
44+
sut.start(fixture.context, fixture.listener)
45+
46+
verify(fixture.sensorManager)
47+
.registerListener(eq(sut), eq(fixture.accelerometer), eq(SensorManager.SENSOR_DELAY_NORMAL))
48+
}
49+
50+
@Test
51+
fun `unregisters sensor listener on stop`() {
52+
val sut = fixture.getSut()
53+
sut.start(fixture.context, fixture.listener)
54+
sut.stop()
55+
56+
verify(fixture.sensorManager).unregisterListener(sut)
57+
}
58+
59+
@Test
60+
fun `does not crash when SensorManager is null`() {
61+
whenever(fixture.context.getSystemService(Context.SENSOR_SERVICE)).thenReturn(null)
62+
63+
val sut = fixture.getSut()
64+
sut.start(fixture.context, fixture.listener)
65+
66+
verify(fixture.sensorManager, never()).registerListener(any(), any<Sensor>(), any())
67+
}
68+
69+
@Test
70+
fun `does not crash when accelerometer is null`() {
71+
whenever(fixture.sensorManager.getDefaultSensor(Sensor.TYPE_ACCELEROMETER)).thenReturn(null)
72+
73+
val sut = fixture.getSut()
74+
sut.start(fixture.context, fixture.listener)
75+
76+
verify(fixture.sensorManager, never()).registerListener(any(), any<Sensor>(), any())
77+
}
78+
79+
@Test
80+
fun `triggers listener when shake is detected`() {
81+
// Advance clock so cooldown check (now - 0 > 1000) passes
82+
SystemClock.setCurrentTimeMillis(2000)
83+
84+
val sut = fixture.getSut()
85+
sut.start(fixture.context, fixture.listener)
86+
87+
// Needs at least SHAKE_COUNT_THRESHOLD (2) readings above threshold
88+
val event1 = createSensorEvent(floatArrayOf(30f, 0f, 0f))
89+
sut.onSensorChanged(event1)
90+
val event2 = createSensorEvent(floatArrayOf(30f, 0f, 0f))
91+
sut.onSensorChanged(event2)
92+
93+
verify(fixture.listener).onShake()
94+
}
95+
96+
@Test
97+
fun `does not trigger listener on single shake`() {
98+
val sut = fixture.getSut()
99+
sut.start(fixture.context, fixture.listener)
100+
101+
// A single threshold crossing should not trigger
102+
val event = createSensorEvent(floatArrayOf(30f, 0f, 0f))
103+
sut.onSensorChanged(event)
104+
105+
verify(fixture.listener, never()).onShake()
106+
}
107+
108+
@Test
109+
fun `does not trigger listener below threshold`() {
110+
val sut = fixture.getSut()
111+
sut.start(fixture.context, fixture.listener)
112+
113+
// Gravity only (1G) - no shake
114+
val event = createSensorEvent(floatArrayOf(0f, 0f, SensorManager.GRAVITY_EARTH))
115+
sut.onSensorChanged(event)
116+
117+
verify(fixture.listener, never()).onShake()
118+
}
119+
120+
@Test
121+
fun `does not trigger listener for non-accelerometer events`() {
122+
val sut = fixture.getSut()
123+
sut.start(fixture.context, fixture.listener)
124+
125+
val event = createSensorEvent(floatArrayOf(30f, 0f, 0f), sensorType = Sensor.TYPE_GYROSCOPE)
126+
sut.onSensorChanged(event)
127+
128+
verify(fixture.listener, never()).onShake()
129+
}
130+
131+
@Test
132+
fun `stop without start does not crash`() {
133+
val sut = fixture.getSut()
134+
sut.stop()
135+
}
136+
137+
private fun createSensorEvent(
138+
values: FloatArray,
139+
sensorType: Int = Sensor.TYPE_ACCELEROMETER,
140+
): SensorEvent {
141+
val sensor = mock<Sensor>()
142+
whenever(sensor.type).thenReturn(sensorType)
143+
144+
val constructor = SensorEvent::class.java.getDeclaredConstructor(Int::class.javaPrimitiveType)
145+
constructor.isAccessible = true
146+
val event = constructor.newInstance(values.size)
147+
values.copyInto(event.values)
148+
149+
val sensorField = SensorEvent::class.java.getField("sensor")
150+
sensorField.set(event, sensor)
151+
152+
return event
153+
}
154+
}

0 commit comments

Comments
 (0)