feat: added csv functionalities #2785
Open
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 adds full CSV import/export and real-time data-logging capabilities by introducing a CsvService for file operations and permissions, extending the LuxMeter provider and common scaffold to support recording, integrating recording controls and save dialogs in the LuxMeter screen, and providing new screens for listing, sharing, deleting, importing, and charting logged data. Supporting updates include localization entries, Android storage permissions, and new dependencies. Sequence diagram for LuxMeter data recording and CSV exportsequenceDiagram
actor User
participant LuxMeterScreen
participant LuxMeterStateProvider
participant CsvService
participant Dialog as SaveFileDialog
User->>LuxMeterScreen: Tap record button
LuxMeterScreen->>LuxMeterStateProvider: startRecording()
LuxMeterStateProvider-->>LuxMeterScreen: isRecording = true
User->>LuxMeterScreen: Tap stop button
LuxMeterScreen->>LuxMeterStateProvider: stopRecording()
LuxMeterStateProvider-->>LuxMeterScreen: recordedData
LuxMeterScreen->>Dialog: Show save file dialog
Dialog-->>LuxMeterScreen: fileName
LuxMeterScreen->>CsvService: writeMetaData('luxmeter', data)
LuxMeterScreen->>CsvService: saveCsvFile('luxmeter', fileName, data)
CsvService-->>LuxMeterScreen: File (or null)
LuxMeterScreen-->>User: Show snackbar (success/failure)
Class diagram for CsvService and LuxMeterStateProvider changesclassDiagram
class CsvService {
+Future<Directory> getInstrumentDirectory(String instrumentName)
+Future<void> requestStoragePermission()
+Future<File?> saveCsvFile(String instrumentName, String fileName, List<List<dynamic>> data)
+Future<List<FileSystemEntity>> getSavedFiles(String instrumentName)
+Future<void> deleteFile(String filePath)
+Future<void> deleteAllFiles(String instrumentName)
+Future<void> shareFile(String filePath)
+Future<List<List<dynamic>>?> pickAndReadCsvFile()
+Future<List<List<dynamic>>> readCsvFromFile(File file)
+void writeMetaData(String instrumentName, List<List<dynamic>> data)
}
class LuxMeterStateProvider {
-bool _isRecording
-List<List<dynamic>> _recordedData
-double _recordingStartTime
+bool get isRecording
+void startRecording()
+List<List<dynamic>> stopRecording()
}
CsvService <.. LuxMeterStateProvider : uses
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @Yugesh-Kumar-S - I've reviewed your changes - here's some feedback:
- Extract the filename‐input AlertDialog into a shared widget or helper function to avoid duplicating the save‐file dialog logic across instruments.
- Move the hardcoded record button tooltips (“Start Recording”/“Stop Recording”) into your localization files to ensure they’re fully internationalized.
- Centralize instrument metadata (icon paths, chart axis labels, CSV directory names) in a single config or model instead of using scattered switch statements and magic indices.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Extract the filename‐input AlertDialog into a shared widget or helper function to avoid duplicating the save‐file dialog logic across instruments.
- Move the hardcoded record button tooltips (“Start Recording”/“Stop Recording”) into your localization files to ensure they’re fully internationalized.
- Centralize instrument metadata (icon paths, chart axis labels, CSV directory names) in a single config or model instead of using scattered switch statements and magic indices.
## Individual Comments
### Comment 1
<location> `lib/others/csv_service.dart:120` </location>
<code_context>
+ Future<void> shareFile(String filePath) async {
+ try {
+ final xFile = XFile(filePath);
+ await SharePlus.instance.share(
+ ShareParams(files: [xFile], text: appLocalizations.sharingMessage));
+ } catch (e) {
</code_context>
<issue_to_address>
Use of SharePlus.instance.share may not be correct for the share_plus package.
Share_plus typically uses Share.share or Share.shareXFiles, not SharePlus.instance.share. Please verify the correct API in the documentation and update the method call.
</issue_to_address>
### Comment 2
<location> `lib/others/csv_service.dart:158` </location>
<code_context>
+ }
+ }
+
+ void writeMetaData(String instrumentName, List<List<dynamic>> data) {
+ final now = DateTime.now();
+ final sdf = DateFormat('yyyy-MM-dd HH:mm:ss');
</code_context>
<issue_to_address>
writeMetaData prepends metadata without checking for existing metadata rows.
If called multiple times, this will add duplicate metadata rows. Please check if the first row is already metadata before prepending.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
void writeMetaData(String instrumentName, List<List<dynamic>> data) {
final now = DateTime.now();
final sdf = DateFormat('yyyy-MM-dd HH:mm:ss');
final metaDataTime = sdf.format(now);
final metaData = [
instrumentName,
metaDataTime.split(' ')[0],
metaDataTime.split(' ')[1]
];
data.insert(0, metaData);
}
=======
void writeMetaData(String instrumentName, List<List<dynamic>> data) {
final now = DateTime.now();
final sdf = DateFormat('yyyy-MM-dd HH:mm:ss');
final metaDataTime = sdf.format(now);
final metaData = [
instrumentName,
metaDataTime.split(' ')[0],
metaDataTime.split(' ')[1]
];
bool isMetaDataRow(List<dynamic> row) {
if (row.length < 3) return false;
if (row[0] != instrumentName) return false;
// Check if row[1] is a date and row[2] is a time in expected format
try {
DateFormat('yyyy-MM-dd').parseStrict(row[1].toString());
DateFormat('HH:mm:ss').parseStrict(row[2].toString());
return true;
} catch (_) {
return false;
}
}
if (data.isEmpty || !isMetaDataRow(data[0])) {
data.insert(0, metaData);
}
}
>>>>>>> REPLACE
</suggested_fix>
### Comment 3
<location> `lib/view/logged_data_chart_screen.dart:190` </location>
<code_context>
+ final row = widget.data[i];
+ if (row.length > widget.xDataColumnIndex &&
+ row.length > widget.yDataColumnIndex) {
+ final x = (row[widget.xDataColumnIndex] as num).toDouble();
+ final y = (row[widget.yDataColumnIndex] as num).toDouble();
+ spots.add(FlSpot(x, y));
+ if (y > maxY) maxY = y;
</code_context>
<issue_to_address>
Casting to num may throw if the CSV contains strings or nulls.
Add error handling or parsing to manage non-numeric values in the CSV, so the code doesn't throw when encountering invalid data.
</issue_to_address>
### Comment 4
<location> `lib/providers/luxmeter_state_provider.dart:118` </location>
<code_context>
+ final relativeTime = time - _recordingStartTime;
+ final now = DateTime.now();
+ final dateFormat = DateFormat('yyyy-MM-dd HH:mm:ss.SSS');
+ _recordedData.add([
+ dateFormat.format(now),
+ relativeTime.toStringAsFixed(2),
+ lux.toStringAsFixed(2),
+ ]);
+ }
</code_context>
<issue_to_address>
Recording uses formatted strings instead of raw values.
Storing formatted strings can hinder future data processing. Store raw numeric values and format them only when displaying or exporting.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
final relativeTime = time - _recordingStartTime;
final now = DateTime.now();
final dateFormat = DateFormat('yyyy-MM-dd HH:mm:ss.SSS');
_recordedData.add([
dateFormat.format(now),
relativeTime.toStringAsFixed(2),
lux.toStringAsFixed(2),
]);
=======
final relativeTime = time - _recordingStartTime;
final now = DateTime.now();
_recordedData.add([
now,
relativeTime,
lux,
]);
>>>>>>> REPLACE
</suggested_fix>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+115
to
+122
| final relativeTime = time - _recordingStartTime; | ||
| final now = DateTime.now(); | ||
| final dateFormat = DateFormat('yyyy-MM-dd HH:mm:ss.SSS'); | ||
| _recordedData.add([ | ||
| dateFormat.format(now), | ||
| relativeTime.toStringAsFixed(2), | ||
| lux.toStringAsFixed(2), | ||
| ]); |
There was a problem hiding this comment.
suggestion: Recording uses formatted strings instead of raw values.
Storing formatted strings can hinder future data processing. Store raw numeric values and format them only when displaying or exporting.
Suggested change
| final relativeTime = time - _recordingStartTime; | |
| final now = DateTime.now(); | |
| final dateFormat = DateFormat('yyyy-MM-dd HH:mm:ss.SSS'); | |
| _recordedData.add([ | |
| dateFormat.format(now), | |
| relativeTime.toStringAsFixed(2), | |
| lux.toStringAsFixed(2), | |
| ]); | |
| final relativeTime = time - _recordingStartTime; | |
| final now = DateTime.now(); | |
| _recordedData.add([ | |
| now, | |
| relativeTime, | |
| lux, | |
| ]); |
|
Build successful. APKs to test: https://github.com/fossasia/pslab-android/actions/runs/16248530925/artifacts/3521285329 |
CloudyPadmal
approved these changes
Jul 13, 2025
Yugesh-Kumar-S
added
Status: Review Required
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.
Fixes #2782
Changes
Screenshots / Recordings
screen-20250713-010559.mp4
Checklist:
strings.xml,dimens.xmlandcolors.xmlwithout hard coding any value.strings.xml,dimens.xmlorcolors.xml.Summary by Sourcery
Introduce end-to-end CSV logging for instruments by adding a CsvService, recording controls in the LuxMeter UI, and new screens to manage, import, and visualize CSV logs, along with required dependencies, permissions, and localization updates
New Features:
Enhancements:
Build: