-
Notifications
You must be signed in to change notification settings - Fork 810
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: app crashes if data is read from sensor which is not connected #2605
Conversation
Previously it was assumed that dat was read successfully if the method was called at least once.
Reviewer's Guide by SourceryThis pull request fixes a crash that occurs when attempting to read data from a disconnected sensor. It modifies the Sequence diagram for sensor data acquisition and UI update flowsequenceDiagram
participant App
participant SensorDataFetch
participant Sensor
participant UI
App->>SensorDataFetch: execute()
SensorDataFetch->>Sensor: getSensorData()
alt Sensor connected and data read successful
Sensor-->>SensorDataFetch: return sensor data
SensorDataFetch->>SensorDataFetch: return true
SensorDataFetch->>UI: updateUI()
UI-->>SensorDataFetch: UI updated
else Sensor disconnected or read failed
Sensor-->>SensorDataFetch: return null/throw error
SensorDataFetch->>SensorDataFetch: return false
Note over SensorDataFetch,UI: Skip UI update
end
Class diagram showing AbstractSensorActivity and SensorDataFetch relationshipclassDiagram
class AbstractSensorActivity {
#getStartTime()
#onOptionsItemSelected()
}
class SensorDataFetch {
#getTimeElapsed()
#execute()
+getSensorData() boolean
+updateUi()
}
AbstractSensorActivity *-- SensorDataFetch : contains
note for SensorDataFetch "Modified to return boolean\nindicating success/failure\nof sensor data acquisition"
State diagram for sensor data acquisition processstateDiagram-v2
[*] --> AttemptRead
AttemptRead --> ReadSuccess: Sensor connected
AttemptRead --> ReadFailure: Sensor disconnected
ReadSuccess --> UpdateUI: Update displays and charts
ReadFailure --> Skip: Skip UI update
UpdateUI --> [*]
Skip --> [*]
note right of ReadFailure: New error handling
note right of Skip: Prevents crash
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @marcnause - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider adding null checks in updateUI() methods to avoid potential NPEs if sensor data objects are null after failed reads
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟡 Complexity: 1 issue found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Build successful. APKs to test: https://github.com/fossasia/pslab-android/actions/runs/12614694571/artifacts/2386467983 |
Changing this PR to use squash merge. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@marcnause Everything looks good to me and the crash is fixed !
Fixes #2604
Changes
Screenshots / Recordings
N/A
Checklist:
strings.xml
,dimens.xml
andcolors.xml
without hard coding any value.strings.xml
,dimens.xml
orcolors.xml
.Summary by Sourcery
Bug Fixes: