This repository has been archived by the owner on Oct 16, 2023. It is now read-only.
[BM-667] Feedback - baby device side #263
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Added checking the frequency of feedback requests Added sending feedback requests with the notifications Added tests
| @@ -47,6 +47,11 @@ class BabyMonitorMessagingService : FirebaseMessagingService() { | |||
| message.data | |||
| ) | |||
| NotificationType.LOW_BATTERY_NOTIFICATION -> handleLowBatteryNotification(message.data) | |||
| NotificationType.CRY_NOTIFICATION_WITH_FEEDBACK_REQUEST, | |||
| NotificationType.NOISE_NOTIFICATION_WITH_FEEDBACK_REQUEST -> { | |||
| // TODO handle notification with feedback request | |||
There was a problem hiding this comment.
| This comment contains text that has been defined as forbidden in detekt. |
| @@ -15,14 +17,30 @@ class NotifyBabyEventUseCase( | |||
| private val babyEvents: PublishSubject<BabyEvent> = PublishSubject.create() | |||
|
|
|||
| private fun fetchClientsAndPostNotification(babyEvent: BabyEvent): Completable { | |||
There was a problem hiding this comment.
| The function fetchClientsAndPostNotification appears to be too complex. |
| Timber.d("Notification posted: $response.") | ||
| } | ||
| .ignoreElement() | ||
| } | ||
|
|
||
| private fun updateFeedbackData(notificationData: NotificationData) { | ||
| feedbackFrequencyUseCase.notificationSent() | ||
| if (notificationData.feedbackRecordingFile.isNotEmpty()) |
There was a problem hiding this comment.
| Multi-line if statement was found that does not have braces. These should be added to improve readability. |
| private fun updateFeedbackData(notificationData: NotificationData) { | ||
| feedbackFrequencyUseCase.notificationSent() | ||
| if (notificationData.feedbackRecordingFile.isNotEmpty()) | ||
| feedbackFrequencyUseCase.feedbackRequestSent() |
| title: String, | ||
| text: String, | ||
| notificationType: NotificationType | ||
| notificationData: NotificationData | ||
| ): Single<Response> { | ||
| return checkIdCall(firebaseToken) | ||
| .flatMap { isAndroid -> | ||
| return@flatMap notificationCall( |
There was a problem hiding this comment.
| Expression with labels increase complexity and affect maintainability. |
| Timber.d("Sound Db: $decibels") | ||
| return@fromCallable decibels.roundToInt() | ||
| Timber.d("Max Db: $maxDecibels") | ||
| return@fromCallable maxDecibels?.roundToInt() ?: 0 |
There was a problem hiding this comment.
| Expression with labels increase complexity and affect maintainability. |
| } | ||
|
|
||
| private fun mapToDecibels(shorts: List<Short>): Double { | ||
| var totalSquared = 0.0 |
There was a problem hiding this comment.
| Unexpected indentation (7) (it should be 8) |
| @@ -26,10 +29,11 @@ class RecordingController @Inject constructor( | |||
| .map { | |||
| val data = when { | |||
| isMachineLearningWithEnoughData(it) -> RecordingData.MachineLearning( | |||
| it.first.takeLast(MachineLearning.DATA_SIZE * 2).toByteArray(), | |||
| it.first.takeLast(FeedbackController.DATA_SIZE * 2).toByteArray(), | |||
There was a problem hiding this comment.
| This expression contains a magic number. Consider defining it to a well named constant. |
| it.second.takeLast(MachineLearning.DATA_SIZE).toShortArray() | ||
| ) | ||
| isNoiseDetectionWithEnoughData(it) -> RecordingData.NoiseDetection( | ||
| it.first.takeLast(FeedbackController.DATA_SIZE * 2).toByteArray(), |
There was a problem hiding this comment.
| This expression contains a magic number. Consider defining it to a well named constant. |
| ) = Single.fromCallable { | ||
| // TODO add metadata to the recordings (should be consulted with ML dev) |
There was a problem hiding this comment.
| This comment contains text that has been defined as forbidden in detekt. |
| .subscribeOn(Schedulers.io()) | ||
| .subscribeBy( | ||
| onSuccess = { savedRecordingData -> | ||
| Timber.i("Recording saved: ${savedRecordingData.fileName}" + |
There was a problem hiding this comment.
| Replace String concatenation with Timber’s string formatting |
| .subscribeOn(Schedulers.io()) | ||
| .subscribeBy( | ||
| onSuccess = { savedRecordingData -> | ||
| Timber.i("Recording saved: ${savedRecordingData.fileName}" + |
There was a problem hiding this comment.
| Multiple occurrences of the same string literal within a single file detected. |
| .subscribeBy( | ||
| onSuccess = { savedRecordingData -> | ||
| Timber.i("Recording saved: ${savedRecordingData.fileName}" + | ||
| " should ask for feedback: ${savedRecordingData.shouldAskForFeedback}") |
There was a problem hiding this comment.
| Multiple occurrences of the same string literal within a single file detected. |
| .subscribeOn(Schedulers.io()) | ||
| .subscribeBy( | ||
| onSuccess = { savedRecordingData -> | ||
| Timber.i("Recording saved: ${savedRecordingData.fileName}" + |
There was a problem hiding this comment.
| Replace String concatenation with Timber’s string formatting |
| @@ -23,9 +24,9 @@ class NotifyLowBatteryUseCaseTest { | |||
| notifyLowBatteryUseCase.notifyLowBattery(title, text) | |||
|
|
|||
| verify(notificationSender).broadcastNotificationToFcm( | |||
There was a problem hiding this comment.
| Unexpected indentation (11) (it should be 12) |
JaCoCO Code Coverage 20.09% ✅
APK file size
Number of method references
Generated by 🚫 Danger |
Aruziell
approved these changes
Feb 20, 2020
Comment on lines
+15
to
+19
| fun shouldAskForFeedback(): Boolean { | ||
| val currentCounter = sharedPreferences.getInt(NOTIFICATION_COUNTER_KEY, 0) | ||
| val lastFeedbackTimestamp = sharedPreferences.getLong(FEEDBACK_TIMESTAMP_KEY, 0) | ||
| return enoughTimePassed(lastFeedbackTimestamp) && currentCounter > NO_FEEDBACK_COUNTER_LIMIT | ||
| } |
There was a problem hiding this comment.
I wonder if it could be modeled with rx. Something like this, perhaps?
fun feedbackRecordings(recordings: Observable<Recording>) =
Observable.interval(DELAY_HOURS, TimeUnit.HOURS)
.withLatestFrom(recordings) { _, recording -> recording }
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Task
Description
Added checking the frequency of feedback requests
Added sending feedback requests with the notifications
Added tests