-
Notifications
You must be signed in to change notification settings - Fork 825
feat: Added experiment framework and created light intensity vs distance experiment. #2857
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
Conversation
Reviewer's GuideThis PR introduces a generic experiment framework with models and a provider to orchestrate multiscreen experiments, implements a Light Intensity vs Distance experiment with condition-based step logic, adds dedicated experiment navigation and guide screens, augments the LuxMeterScreen to run experiments via a draggable overlay, and updates localization and theme resources accordingly. Sequence diagram for experiment step condition checking in LuxMeterScreensequenceDiagram
participant LuxMeterScreen
participant ExperimentProvider
participant User as actor
loop Every second (if isExperiment)
LuxMeterScreen->>ExperimentProvider: checkStepCondition(luxData, timeData)
alt Step condition met
ExperimentProvider->>LuxMeterScreen: notifies step completion
LuxMeterScreen->>User: shows "Step Completed" overlay
ExperimentProvider->>ExperimentProvider: nextStep() after delay
else Step not met
LuxMeterScreen->>User: shows current instruction overlay
end
end
Class diagram for the new experiment frameworkclassDiagram
class ExperimentConfig {
+String id
+String title
+String description
+IconData icon
+List<Map<String, String>> guideSteps
+List<ExperimentStep> experimentSteps
+String targetScreen
}
class ExperimentStep {
+String id
+String instruction
+Duration? timeout
+bool checkCondition(List<double> values, List<double> timeData)
}
class ExperimentProvider {
-ExperimentConfig? _currentExperiment
-int _currentStepIndex
-bool _isExperimentActive
-bool _isStepCompleted
-Timer? _stepTimer
-DateTime? _stepStartTime
-bool _isExperimentCompleted
+startExperiment(ExperimentConfig experiment)
+checkStepCondition(List<double> values, List<double> timeData)
+nextStep()
+stopExperiment()
}
ExperimentConfig "1" *-- "many" ExperimentStep
ExperimentProvider "1" o-- "1" ExperimentConfig
ExperimentProvider "1" o-- "1" ExperimentStep
Class diagram for Light Intensity vs Distance experiment stepsclassDiagram
class MoveTowardsLightStep {
+checkCondition(List<double> values, List<double> timeData)
}
class MoveAwayFromLightStep {
+checkCondition(List<double> values, List<double> timeData)
}
class StabilizeReadingStep {
+checkCondition(List<double> values, List<double> timeData)
}
MoveTowardsLightStep --|> ExperimentStep
MoveAwayFromLightStep --|> ExperimentStep
StabilizeReadingStep --|> ExperimentStep
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Build StatusBuild successful. APKs to test: https://github.com/fossasia/pslab-app/actions/runs/17190909185/artifacts/3838234406. Screenshots |
|
@sourcery-ai review |
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 there - I've reviewed your changes - here's some feedback:
- Wrap ExperimentGuideScreen with a ChangeNotifierProvider for ExperimentProvider (and ensure it’s properly disposed) instead of instantiating it manually in initState to avoid potential memory leaks.
- Refactor the repeated MaterialPageRoute logic in ExperimentsScreen (_buildExperimentCard and button onPressed) into a shared helper to reduce duplication.
- Split the large ExperimentOverlayWidget build method into smaller widgets or helper methods to improve readability and maintainability.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Wrap ExperimentGuideScreen with a ChangeNotifierProvider for ExperimentProvider (and ensure it’s properly disposed) instead of instantiating it manually in initState to avoid potential memory leaks.
- Refactor the repeated MaterialPageRoute logic in ExperimentsScreen (_buildExperimentCard and button onPressed) into a shared helper to reduce duplication.
- Split the large ExperimentOverlayWidget build method into smaller widgets or helper methods to improve readability and maintainability.
## Individual Comments
### Comment 1
<location> `lib/providers/experiment_provider.dart:39` </location>
<code_context>
+ void checkStepCondition(List<double> values, List<double> timeData) {
+ if (!_isExperimentActive || _isStepCompleted || currentStep == null) return;
+
+ if (_stepStartTime != null &&
+ DateTime.now().difference(_stepStartTime!).inSeconds < 3) {
+ return;
</code_context>
<issue_to_address>
Step condition check includes a hardcoded 3-second delay.
Consider making the delay configurable or documenting the rationale for using a fixed 3-second value.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
void checkStepCondition(List<double> values, List<double> timeData) {
if (!_isExperimentActive || _isStepCompleted || currentStep == null) return;
if (_stepStartTime != null &&
DateTime.now().difference(_stepStartTime!).inSeconds < 3) {
return;
}
=======
/// The minimum delay (in seconds) before step condition is checked.
/// Default is 3 seconds to allow system stabilization after step start.
int stepConditionDelaySeconds = 3;
void checkStepCondition(List<double> values, List<double> timeData) {
if (!_isExperimentActive || _isStepCompleted || currentStep == null) return;
if (_stepStartTime != null &&
DateTime.now().difference(_stepStartTime!).inSeconds < stepConditionDelaySeconds) {
return;
}
>>>>>>> REPLACE
</suggested_fix>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
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.
Sorry @Yugesh-Kumar-S, you have reached your weekly rate limit for Sourcery. Please try again later
AsCress
left a comment
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.
|
@Yugesh-Kumar-S Nice job ! Please resolve conflicts here. |
|
@Yugesh-Kumar-S Can you please document somewhere how you created the GIF files? I think it would be very useful to know how you created them if somebody else wants to add experiments at some point in the future. If this is too much additional effort or if you think it is out of scope for this PR, please open a new issue. |







-1_instruments_screen.png?raw=true)
-2_nav_drawer.png?raw=true)
-3_accelerometer.png?raw=true)
-4_power_source.png?raw=true)
-5_multimeter.png?raw=true)
-6_wave_generator.png?raw=true)
-7_oscilloscope.png?raw=true)
Fixes #2797
Changes
Screenshots / Recordings
screen-20250820-002933.mp4
Checklist:
strings.xml,dimens.xmlandcolors.xmlwithout hard coding any value.strings.xml,dimens.xmlorcolors.xml.Summary by Sourcery
Introduce a new experiment framework and create a Light Intensity vs Distance experiment with UI integration and localization support.
New Features:
Enhancements:
Summary by Sourcery
Add a modular experiment framework and integrate a new Light Intensity vs Distance experiment with guided steps, overlay instructions, and localization, along with UI and navigation enhancements.
New Features:
Enhancements: