feat: added functionality for Wave Generator #2826
Open
+1,652
−322
Conversation
This file contains hidden or 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
Reviewer's GuideThis PR overhauls the Wave Generator feature by introducing a ChangeNotifier-based state provider with strongly typed enums for wave parameters, extending the ScienceLab communication layer with new waveform setup methods, and refactoring the UI to consume provider state and localized labels dynamically. Sequence diagram for updating wave parameter via providersequenceDiagram
actor User
participant UI
participant WaveGeneratorStateProvider
participant ScienceLab
User->>UI: Adjusts wave parameter (e.g., frequency)
UI->>WaveGeneratorStateProvider: setValue(newValue)
WaveGeneratorStateProvider->>ScienceLab: setWave() (calls appropriate waveform method)
ScienceLab-->>WaveGeneratorStateProvider: Updates hardware
WaveGeneratorStateProvider->>UI: Notifies listeners
UI->>UI: Updates display
Class diagram for Wave Generator state managementclassDiagram
class WaveGeneratorStateProvider {
- int sin
- int triangular
- int pwm
- WaveConst? selectedAnalogWave
- WaveConst? selectedDigitalWave
- WaveConst? propSelected
- WaveGeneratorConstants waveGeneratorConstants
- List<List<FlSpot>> waveData
- ScienceLab _scienceLab
+ setAnalogSelectedWave(WaveConst)
+ setDigitalSelectedWave(WaveConst)
+ setPropSelected(WaveConst)
+ setAnalogWaveType(int)
+ setValue(int)
+ setWave()
+ incrementValue()
+ decrementValue()
+ previewWave()
+ getSamplePoints(bool)
+ createPlots()
}
class WaveGeneratorConstants {
+ Map<WaveConst, Map<WaveConst, int>> wave
+ WaveConst modeSelected
+ Map<String, int> state
}
class WaveConst
class WaveData {
freqMin
dutyMin
phaseMin
freqMax
phaseMax
dutyMax
value
getValue
}
WaveGeneratorStateProvider --> WaveGeneratorConstants
WaveGeneratorConstants --> WaveConst
WaveGeneratorConstants --> WaveData
WaveGeneratorStateProvider --> ScienceLab
WaveGeneratorStateProvider --> FlSpot
Class diagram for new ScienceLab waveform methodsclassDiagram
class ScienceLab {
+ setSI1(frequency, waveType)
+ setSI2(frequency, waveType)
+ setWaves(frequency, phase, frequency2)
+ sqrPWM(frequency, h0, p1, h1, p2, h2, p3, h3, pulse)
}
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Build successful. APKs to test: https://github.com/fossasia/pslab-app/actions/runs/17005777364/artifacts/3779054720 |
AsCress
force-pushed
the
wavegen_func
branch
2 times, most recently
from
5b2a328 to
2b7e90b
Compare
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- There’s a lot of duplicated logic in science_lab’s new waveform methods (setSI1, setSI2, setWaves, sqrPWM) — extracting shared parts into helpers or a single configurable routine would reduce repetition.
- The build functions in the waveform control widgets have grown very large with nested ternaries and inline logic — splitting out sub-widgets or helper methods would greatly improve readability.
- Managing wave parameters via nested enum-keyed maps in WaveGeneratorStateProvider/WaveGeneratorConstants is hard to follow; consider modeling each wave’s settings with a dedicated class or data structure instead of deeply nested maps.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- There’s a lot of duplicated logic in science_lab’s new waveform methods (setSI1, setSI2, setWaves, sqrPWM) — extracting shared parts into helpers or a single configurable routine would reduce repetition.
- The build functions in the waveform control widgets have grown very large with nested ternaries and inline logic — splitting out sub-widgets or helper methods would greatly improve readability.
- Managing wave parameters via nested enum-keyed maps in WaveGeneratorStateProvider/WaveGeneratorConstants is hard to follow; consider modeling each wave’s settings with a dedicated class or data structure instead of deeply nested maps.
## Individual Comments
### Comment 1
<location> `lib/view/widgets/wave_generator_graph.dart:36` </location>
<code_context>
return Container(
padding: const EdgeInsets.only(left: 20, right: 20, bottom: 20),
child: LineChart(
+ duration: const Duration(milliseconds: 10),
LineChartData(
backgroundColor: chartBackgroundColor,
</code_context>
<issue_to_address>
Very short chart animation duration may be imperceptible.
Consider increasing the animation duration or making it configurable to improve user experience.
Suggested implementation:
```
class WaveGeneratorGraph extends StatefulWidget {
final Duration chartAnimationDuration;
const WaveGeneratorGraph({
Key? key,
this.chartAnimationDuration = const Duration(milliseconds: 300),
}) : super(key: key);
@override
Widget build(BuildContext context) {
```
```
return Container(
padding: const EdgeInsets.only(left: 20, right: 20, bottom: 20),
child: LineChart(
duration: chartAnimationDuration,
LineChartData(
backgroundColor: chartBackgroundColor,
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
@marcnause @CloudyPadmal This is now complete and ready to go in. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Adds the functionality for the Wave Generator instrument.
Screenshots / Recordings
Screen_recording_20250815_234703.mp4
Screen_recording_20250815_235536.mp4
Screen_recording_20250816_124758.mp4
Summary by Sourcery
Introduce state management for the Wave Generator instrument with enums for constants and data limits and a ChangeNotifier provider to track selected wave types.
New Features: