diff --git a/AGENTS.md b/AGENTS.md index fad3dc5a54..42a8e65100 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -144,6 +144,17 @@ The repository is organized into multiple modules: 4. New features must be **opt-in by default** - extend `SentryOptions` or similar Option classes with getters/setters 5. Consider backwards compatibility +### Third-Party Code Attribution +When adapting code from third-party libraries: +1. Add a license header at the top of the adapted file (before the `package` statement): + ```java + // Adapted from . + // Copyright . + // Licensed under the . + // + ``` +2. Add a full attribution entry to `THIRD_PARTY_NOTICES.md` following the existing format (Source, License, Copyright, Scope, full license text) + ### Getting PR Information Use `gh pr view` to get PR details from the current branch. This is needed when adding changelog entries, which require the PR number. diff --git a/CHANGELOG.md b/CHANGELOG.md index 042e87b971..d1781ad040 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,8 @@ ### Fixes - Fix soft input keyboard not being shown on the Feedback form ([#5359](https://github.com/getsentry/sentry-java/pull/5359)) +- Fix shake-to-report not triggering on some devices due to high acceleration threshold ([#5366](https://github.com/getsentry/sentry-java/pull/5366)) +- Fix feedback form retaining previous message when shown again via shake ([#5366](https://github.com/getsentry/sentry-java/pull/5366)) ### Dependencies diff --git a/THIRD_PARTY_NOTICES.md b/THIRD_PARTY_NOTICES.md index 8b2141cc59..5a48d567fa 100644 --- a/THIRD_PARTY_NOTICES.md +++ b/THIRD_PARTY_NOTICES.md @@ -118,6 +118,34 @@ limitations under the License. --- +## Square — Seismic (Apache 2.0) + +**Source:** https://github.com/square/seismic
+**License:** Apache License 2.0
+**Copyright:** Copyright 2010 Square, Inc. + +### Scope + +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`. + +``` +Copyright 2010 Square, Inc. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +``` + +--- + ## Square — Curtains (Apache 2.0) **Source:** https://github.com/square/curtains (v1.2.5)
diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SentryShakeDetector.java b/sentry-android-core/src/main/java/io/sentry/android/core/SentryShakeDetector.java index 5b6f63309f..a4c4ae0c4f 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/SentryShakeDetector.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SentryShakeDetector.java @@ -1,3 +1,7 @@ +// Adapted from Square's Seismic library. +// Copyright 2010 Square, Inc. +// Licensed under the Apache License, Version 2.0. +// https://github.com/square/seismic package io.sentry.android.core; import android.content.Context; @@ -7,10 +11,8 @@ import android.hardware.SensorManager; import android.os.Handler; import android.os.HandlerThread; -import android.os.SystemClock; import io.sentry.ILogger; import io.sentry.SentryLevel; -import java.util.concurrent.atomic.AtomicLong; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -21,8 +23,8 @@ *

The accelerometer sensor (TYPE_ACCELEROMETER) does NOT require any special permissions on * Android. The BODY_SENSORS permission is only needed for heart rate and similar body sensors. * - *

Requires at least {@link #SHAKE_COUNT_THRESHOLD} accelerometer readings above {@link - * #SHAKE_THRESHOLD_GRAVITY} within {@link #SHAKE_WINDOW_MS} to trigger a shake event. + *

Uses a rolling sample window: if more than 75% of accelerometer readings in the past 0.5s + * exceed {@link #ACCELERATION_THRESHOLD}, a shake is detected. Based on Square's Seismic library. * *

Sensor events are delivered on a background {@link HandlerThread} to avoid polluting the main * thread. @@ -30,21 +32,16 @@ @ApiStatus.Internal public final class SentryShakeDetector implements SensorEventListener { - private static final float SHAKE_THRESHOLD_GRAVITY = 2.7f; - private static final int SHAKE_WINDOW_MS = 1500; - private static final int SHAKE_COUNT_THRESHOLD = 2; - private static final int SHAKE_COOLDOWN_MS = 1000; + static final int ACCELERATION_THRESHOLD = 13; private @Nullable SensorManager sensorManager; private @Nullable Sensor accelerometer; private @Nullable HandlerThread handlerThread; private @Nullable Handler handler; - private final @NotNull AtomicLong lastShakeTimestamp = new AtomicLong(0); private volatile @Nullable Listener listener; private @NotNull ILogger logger; - private int shakeCount = 0; - private long firstShakeTimestamp = 0; + private final @NotNull SampleQueue queue = new SampleQueue(); public interface Listener { void onShake(); @@ -94,17 +91,24 @@ public void start(final @NotNull Context context, final @NotNull Listener shakeL public void stop() { listener = null; - shakeCount = 0; - firstShakeTimestamp = 0; if (sensorManager != null) { sensorManager.unregisterListener(this); } + final @Nullable Handler h = handler; + if (h != null) { + h.post( + () -> { + //noinspection Convert2MethodRef + queue.clear(); + }); + } } /** Stops detection and releases the background thread. */ public void close() { stop(); if (handlerThread != null) { + // quitSafely drains pending messages (including the clear posted by stop) before exiting handlerThread.quitSafely(); handlerThread = null; handler = null; @@ -116,32 +120,17 @@ public void onSensorChanged(final @NotNull SensorEvent event) { if (event.sensor.getType() != Sensor.TYPE_ACCELEROMETER) { return; } - float gX = event.values[0] / SensorManager.GRAVITY_EARTH; - float gY = event.values[1] / SensorManager.GRAVITY_EARTH; - float gZ = event.values[2] / SensorManager.GRAVITY_EARTH; - double gForceSquared = gX * gX + gY * gY + gZ * gZ; - if (gForceSquared > SHAKE_THRESHOLD_GRAVITY * SHAKE_THRESHOLD_GRAVITY) { - long now = SystemClock.elapsedRealtime(); - - // Reset counter if outside the detection window - if (now - firstShakeTimestamp > SHAKE_WINDOW_MS) { - shakeCount = 0; - firstShakeTimestamp = now; - } - - shakeCount++; - - if (shakeCount >= SHAKE_COUNT_THRESHOLD) { - // Enforce cooldown so we don't fire repeatedly - long lastShake = lastShakeTimestamp.get(); - if (now - lastShake > SHAKE_COOLDOWN_MS) { - lastShakeTimestamp.set(now); - shakeCount = 0; - final @Nullable Listener currentListener = listener; - if (currentListener != null) { - currentListener.onShake(); - } - } + final float ax = event.values[0]; + final float ay = event.values[1]; + final float az = event.values[2]; + final boolean accelerating = Math.sqrt(ax * ax + ay * ay + az * az) > ACCELERATION_THRESHOLD; + + queue.add(event.timestamp, accelerating); + if (queue.isShaking()) { + queue.clear(); + final @Nullable Listener currentListener = listener; + if (currentListener != null) { + currentListener.onShake(); } } } @@ -150,4 +139,97 @@ public void onSensorChanged(final @NotNull SensorEvent event) { public void onAccuracyChanged(final @NotNull Sensor sensor, final int accuracy) { // Not needed for shake detection. } + + static class SampleQueue { + private static final long MAX_WINDOW_SIZE_NS = 500_000_000L; // 0.5s + private static final long MIN_WINDOW_SIZE_NS = MAX_WINDOW_SIZE_NS >> 1; // 0.25s + private static final int MIN_QUEUE_SIZE = 4; + + private final @NotNull SamplePool pool = new SamplePool(); + private @Nullable Sample oldest; + private @Nullable Sample newest; + private int sampleCount; + private int acceleratingCount; + + void add(final long timestamp, final boolean accelerating) { + purge(timestamp - MAX_WINDOW_SIZE_NS); + + final @NotNull Sample added = pool.acquire(); + added.timestamp = timestamp; + added.accelerating = accelerating; + added.next = null; + if (newest != null) { + newest.next = added; + } + newest = added; + if (oldest == null) { + oldest = added; + } + + sampleCount++; + if (accelerating) { + acceleratingCount++; + } + } + + void clear() { + while (oldest != null) { + final @NotNull Sample removed = oldest; + oldest = removed.next; + pool.release(removed); + } + newest = null; + sampleCount = 0; + acceleratingCount = 0; + } + + private void purge(final long cutoff) { + while (sampleCount >= MIN_QUEUE_SIZE && oldest != null && cutoff - oldest.timestamp > 0) { + final @NotNull Sample removed = oldest; + if (removed.accelerating) { + acceleratingCount--; + } + sampleCount--; + oldest = removed.next; + if (oldest == null) { + newest = null; + } + pool.release(removed); + } + } + + boolean isShaking() { + return newest != null + && oldest != null + && sampleCount >= MIN_QUEUE_SIZE + && newest.timestamp - oldest.timestamp >= MIN_WINDOW_SIZE_NS + && acceleratingCount >= (sampleCount >> 1) + (sampleCount >> 2); + } + } + + static class Sample { + long timestamp; + boolean accelerating; + @Nullable Sample next; + } + + static class SamplePool { + private @Nullable Sample head; + + @NotNull + Sample acquire() { + Sample acquired = head; + if (acquired == null) { + acquired = new Sample(); + } else { + head = acquired.next; + } + return acquired; + } + + void release(final @NotNull Sample sample) { + sample.next = head; + head = sample; + } + } } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SentryUserFeedbackForm.java b/sentry-android-core/src/main/java/io/sentry/android/core/SentryUserFeedbackForm.java index 2800d5670a..43500d50eb 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/SentryUserFeedbackForm.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SentryUserFeedbackForm.java @@ -324,6 +324,12 @@ public void setOnDismissListener(final @Nullable OnDismissListener listener) { @Override protected void onStart() { super.onStart(); + // Clear the message field so subsequent show() calls start with a fresh form + final @NotNull EditText edtMessage = + findViewById(R.id.sentry_dialog_user_feedback_edt_description); + edtMessage.getText().clear(); + edtMessage.setError(null); + final @NotNull SentryOptions options = Sentry.getCurrentScopes().getOptions(); final @NotNull SentryFeedbackOptions feedbackOptions = options.getFeedbackOptions(); final @Nullable Runnable onFormOpen = feedbackOptions.getOnFormOpen(); diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/SentryShakeDetectorTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/SentryShakeDetectorTest.kt index 98441e48a8..24ccfceaa8 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/SentryShakeDetectorTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/SentryShakeDetectorTest.kt @@ -5,10 +5,11 @@ import android.hardware.Sensor import android.hardware.SensorEvent import android.hardware.SensorManager import android.os.Handler -import android.os.SystemClock import androidx.test.ext.junit.runners.AndroidJUnit4 import io.sentry.ILogger import kotlin.test.Test +import kotlin.test.assertFalse +import kotlin.test.assertTrue import org.junit.runner.RunWith import org.mockito.kotlin.any import org.mockito.kotlin.eq @@ -88,29 +89,27 @@ class SentryShakeDetectorTest { } @Test - fun `triggers listener when shake is detected`() { - // Advance clock so cooldown check (now - 0 > 1000) passes - SystemClock.setCurrentTimeMillis(2000) - + fun `triggers listener when sustained shake is detected`() { val sut = fixture.getSut() sut.start(fixture.context, fixture.listener) - // Needs at least SHAKE_COUNT_THRESHOLD (2) readings above threshold - val event1 = createSensorEvent(floatArrayOf(30f, 0f, 0f)) - sut.onSensorChanged(event1) - val event2 = createSensorEvent(floatArrayOf(30f, 0f, 0f)) - sut.onSensorChanged(event2) + // Send enough accelerating samples over 0.25s+ to trigger (>75% accelerating) + val baseTimestamp = 1_000_000_000L // 1s in nanos + val intervalNs = 20_000_000L // 20ms between samples (~50Hz) + for (i in 0 until 20) { + val event = createSensorEvent(floatArrayOf(20f, 0f, 0f), baseTimestamp + i * intervalNs) + sut.onSensorChanged(event) + } verify(fixture.listener).onShake() } @Test - fun `does not trigger listener on single shake`() { + fun `does not trigger listener on single spike`() { val sut = fixture.getSut() sut.start(fixture.context, fixture.listener) - // A single threshold crossing should not trigger - val event = createSensorEvent(floatArrayOf(30f, 0f, 0f)) + val event = createSensorEvent(floatArrayOf(30f, 0f, 0f), 1_000_000_000L) sut.onSensorChanged(event) verify(fixture.listener, never()).onShake() @@ -121,9 +120,16 @@ class SentryShakeDetectorTest { val sut = fixture.getSut() sut.start(fixture.context, fixture.listener) - // Gravity only (1G) - no shake - val event = createSensorEvent(floatArrayOf(0f, 0f, SensorManager.GRAVITY_EARTH)) - sut.onSensorChanged(event) + val baseTimestamp = 1_000_000_000L + val intervalNs = 20_000_000L + for (i in 0 until 20) { + val event = + createSensorEvent( + floatArrayOf(0f, 0f, SensorManager.GRAVITY_EARTH), + baseTimestamp + i * intervalNs, + ) + sut.onSensorChanged(event) + } verify(fixture.listener, never()).onShake() } @@ -133,7 +139,7 @@ class SentryShakeDetectorTest { val sut = fixture.getSut() sut.start(fixture.context, fixture.listener) - val event = createSensorEvent(floatArrayOf(30f, 0f, 0f), sensorType = Sensor.TYPE_GYROSCOPE) + val event = createSensorEvent(floatArrayOf(30f, 0f, 0f), 1_000_000_000L, Sensor.TYPE_GYROSCOPE) sut.onSensorChanged(event) verify(fixture.listener, never()).onShake() @@ -145,8 +151,53 @@ class SentryShakeDetectorTest { sut.stop() } + @Test + fun `sample queue triggers when 75 percent of samples are accelerating`() { + val queue = SentryShakeDetector.SampleQueue() + val intervalNs = 20_000_000L + + // 15 accelerating + 5 not = 75% in a 0.4s window (> 0.25s minimum) + for (i in 0 until 15) { + queue.add(i * intervalNs, true) + } + for (i in 15 until 20) { + queue.add(i * intervalNs, false) + } + + assertTrue(queue.isShaking()) + } + + @Test + fun `sample queue does not trigger below 75 percent`() { + val queue = SentryShakeDetector.SampleQueue() + val intervalNs = 20_000_000L + + // 10 accelerating + 10 not = 50% + for (i in 0 until 10) { + queue.add(i * intervalNs, true) + } + for (i in 10 until 20) { + queue.add(i * intervalNs, false) + } + + assertFalse(queue.isShaking()) + } + + @Test + fun `sample queue does not trigger below minimum window`() { + val queue = SentryShakeDetector.SampleQueue() + + // All accelerating but only 0.06s apart (below 0.25s minimum) + for (i in 0 until 4) { + queue.add(i * 20_000_000L, true) + } + + assertFalse(queue.isShaking()) + } + private fun createSensorEvent( values: FloatArray, + timestamp: Long = 0L, sensorType: Int = Sensor.TYPE_ACCELEROMETER, ): SensorEvent { val sensor = mock() @@ -160,6 +211,9 @@ class SentryShakeDetectorTest { val sensorField = SensorEvent::class.java.getField("sensor") sensorField.set(event, sensor) + val timestampField = SensorEvent::class.java.getField("timestamp") + timestampField.set(event, timestamp) + return event } }