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.

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

Add Removable Storage Events. #41

Closed
wants to merge 12 commits into from

Conversation

networkfusion
Copy link
Member

@networkfusion networkfusion commented Aug 5, 2021

Description

Motivation and Context

The replacement of Windows.Storage does not include detection of removable devices. Although this is (was) only available with UWP, it is very important in nanoFramework. This PR is a first attempt to work out the best possible approach,

Comments welcome, and extra input relating to nanoframework/Home#799 advisable.

How Has This Been Tested?

None yet. It requires the native implementation to be added!

Screenshots

Types of changes

  • Improvement (non-breaking change that improves a feature, code or algorithm)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • 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 (work on Unit Tests, 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.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@nfbot nfbot added the Type: enhancement New feature or request label Aug 5, 2021
@networkfusion
Copy link
Member Author

Of note> <SpecificVersion>True</SpecificVersion> for nuget packages is required for errors when building locally!

Copy link
Member

@Ellerbach Ellerbach left a comment

Choose a reason for hiding this comment

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

Few comments, I haven't been really using the Windows storage class so I don't really have a strong opinion on all this.
Only strong one is to go for simplicity and harmonize all storage access, including the non removable one.

nanoFramework.Storage/ExternalDriver.cs Outdated Show resolved Hide resolved
}
}

internal static string DriveIndexToPath(byte driveIndex)
Copy link
Member

Choose a reason for hiding this comment

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

I guess this part may not be needed or then better documented. Or at least contains somehow the non removable storage as well. Like C:
It's the moment to simplify all the complexity introduced by the Windows namespace

Copy link
Member Author

Choose a reason for hiding this comment

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

I guess we could use the https://docs.microsoft.com/en-us/dotnet/api/system.io.driveinfo?view=net-5.0 class instead of this (I only learnt about it yesterday). That would also be more fitting with nanoframework/Home#799

Copy link
Member

Choose a reason for hiding this comment

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

Yes, it makes sense. Still, we need the events as they don't seem to exist on any .NET namespaces...

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.

This should be part of System.IO.FileSystem library. Why the need for a separate assembly?

@networkfusion networkfusion mentioned this pull request Aug 12, 2021
14 tasks
@networkfusion
Copy link
Member Author

Closing as superseeded.

@nfbot nfbot added Status: Invalid This doesn't seem right and removed Type: enhancement New feature or request labels Aug 12, 2021
@networkfusion networkfusion deleted the develop-nanoframework-storage branch October 18, 2021 21:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Invalid This doesn't seem right
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants