Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add Storage Operation Commands #374

Merged
merged 14 commits into from
Mar 21, 2024
Merged

Add Storage Operation Commands #374

merged 14 commits into from
Mar 21, 2024

Conversation

Ellerbach
Copy link
Member

@Ellerbach Ellerbach commented Mar 12, 2024 •
edited by josesimoes
Loading

Description

  • Add Storage Operation Command
  • AddFile and RemoveFile support
  • Split the byte array to save as a file in chunks internally and pass it to the device
  • Added 2 buttons to add and remove a file in the WPF test application

Motivation and Context

How Has This Been Tested?

On a real ESP32 device

Screenshots

Types of changes

  • Improvement (non-breaking change that improves a feature, code or algorithm)
  • Bug fix (non-breaking change which fixes an issue with code or algorithm)
  • New feature (non-breaking change which adds functionality to code)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Config and build (change in the configuration and build system, has no impact on code or features)
  • Dependencies (update dependencies and changes associated, has no impact on code or features)
  • Unit Tests (add new Unit Test(s) or improved existing one(s), has no impact on code or features)
  • Documentation (changes or updates in the documentation, has no impact on code or features)

Checklist:

  • My code follows the code style of this project (only if there are changes in source code).
  • My changes require an update to the documentation (there are changes that require the docs website to be updated).
  • I have updated the documentation accordingly (the changes require an update on the docs in this repo).
  • I have read the CONTRIBUTING document.
  • I have tested everything locally and all new and existing tests passed (only if there are changes in source code).
  • I have added new tests to cover my changes.

@Ellerbach Ellerbach marked this pull request as ready for review March 13, 2024 15:20
@josesimoes josesimoes mentioned this pull request Mar 20, 2024
Draft
14 tasks
@josesimoes josesimoes mentioned this pull request Mar 20, 2024
Merged
13 tasks
josesimoes and others added 13 commits March 20, 2024 20:32
@josesimoes
Add command to perform operations on storage
02a799b 
- Add class to support the command in Wire Protocol.
@Ellerbach @josesimoes
adding minimum elements
b1f3a79
@josesimoes
Add command to perform operations on storage
cac1ca0 
- Add class to support the command in Wire Protocol.
@Ellerbach @josesimoes
Working version
f4e64bd
@josesimoes
Rename enum for clarity
d1bdee9 
- Also added/improve Intellisense comments.
@josesimoes
Rework Monitor_StorageOperation
455b82c 
- Add offset field to support memory access storage.
- Add Intellisense comments.
@josesimoes
Rework PrepareForSend in Monitor_StorageOperation
562aaf5 
- Logic simplification as all operations require sending the file name and operation code.
- Add Intellisense comments.
@josesimoes
Rework storage operations
99ac7e8 
- Aligning with changes in command.
- Rename methods to add Storage for clarity.
- AddStorageFile now instantiates a new Monitor_StorageOperation for each operation.
- Add Intellisense comments to methods.
@josesimoes
Code style fixes
39050d2
@josesimoes
Add PlatformError to StorageOperationErrorCode
90a819d
@josesimoes
Rework Monitor_StorageOperation class
3b1a2c9 
- Add new property to store file name as string.
- PrepareForSend now assembles data buffer with file name and data.
@josesimoes
Fix method names in test app
6c04fe7
@josesimoes
Add PrepareForSend to deal with "no data" operations
e4ba0de
josesimoes
josesimoes approved these changes Mar 20, 2024
View reviewed changes
Copy link
Member

@josesimoes josesimoes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@josesimoes
Copy link
Member

The only thing missing here is the progress report, just like the other ones that are using during deployment to provide feedback to user. It's probably worth adding it now as each release of the debugger touches a lot of dependents...

@Ellerbach
Copy link
Member Author

The only thing missing here is the progress report, just like the other ones that are using during deployment to provide feedback to user. It's probably worth adding it now as each release of the debugger touches a lot of dependents...

For this, the 2 new functions are either success either failure, I feel it's enough to have. You want something that shows progress on chunk upload?

@josesimoes
Copy link
Member

Depends on how large the files are, of course. But there will always by time involved and, just like we do with all the rest we should provide feedback to the user.
It's no big deal, I'll follow the progress pattern (and instrumentation interface) and will add that.

@josesimoes
Add progress report to storage operation handlers
2121025 
- Add new parameters to allow report progress when executing operations.
- Rework processing of reply to properly report errors.
- Rework also about processing reply when target doesn't have access to storage, so this is reported properly.
@Ellerbach
Copy link
Member Author

All seems good to me. Thanks for moving forward the code!

@josesimoes
Copy link
Member

Thank YOU for getting all this started.

@josesimoes josesimoes merged commit e9212f7 into main Mar 21, 2024
3 checks passed
@josesimoes josesimoes changed the title Add Storage Operation Command Add Storage Operation Commands Mar 21, 2024
@josesimoes josesimoes deleted the add-storageoperation-command branch March 21, 2024 12:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@josesimoes josesimoes josesimoes approved these changes

Assignees
No one assigned
Labels
CI: Update Dependents Type: enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Ellerbach @josesimoes @nfbot