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 external events and drivers #43

Merged
merged 17 commits into from
Oct 11, 2021
Merged

Conversation

networkfusion
Copy link
Member

@networkfusion networkfusion commented Aug 12, 2021

Description

Superseeds #41. 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. It also includes another class for initializing external devices that are not part of the native framework.

Uses namespace System.IO.nanoFramework for ExternalDrivers and Events.

Motivation and Context

Adds support for removable devices and external driver support.

How Has This Been Tested?

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 12, 2021
Attempts to find issues with build!
Removes invalid solution project defefinition.
@josesimoes josesimoes force-pushed the develop-removable-events branch 2 times, most recently from 48120b0 to 81c32a8 Compare August 26, 2021 10:59
- Rename namespace.
- Rename class.
- Add data width enum to make the parameter clear (same weight on interface size and processing for native code).
- Split MountSpi method to make it clear that an fixed configuration option is available.
- Adjusted documentation.
- Rework using for System.Diagnostics namespace.
@josesimoes josesimoes force-pushed the develop-removable-events branch from 81c32a8 to 72c34f9 Compare August 26, 2021 10:59
@josesimoes
Copy link
Member

josesimoes commented Aug 26, 2021

@networkfusion this is building again. The failure was being caused by using a static using .

Actually is not on the pipeline... I'll have to figure out why... 😞

@josesimoes
Copy link
Member

@networkfusion @Ellerbach @AdrianSoundy please review and comment the revied API in SDCard class.

@AdrianSoundy
Copy link
Member

Tried to do some testing with this but it's not building. RemovableDeviceEvent not found. Looks like it in wrong position in code.
Moved it but then got a strange MDP error, so gave up
@josesimoes

Source code changes looked ok.

@josesimoes
Copy link
Member

@networkfusion @AdrianSoundy issue related with MDP fixed. This is now building OK. Really really OK 😅 !

@AdrianSoundy
Copy link
Member

AdrianSoundy commented Sep 3, 2021

Ok i have it building now. I'll see if a can do some testing in next few days.

@AdrianSoundy
Copy link
Member

AdrianSoundy commented Sep 6, 2021

Is there any native code for these changes ? I couldn't find them.
@josesimoes @networkfusion

@josesimoes
Copy link
Member

Is there any native code for these changes ? I couldn't find them.
@josesimoes @networkfusion

Nope. Robin was adjusting the C# to bring in the "missing" classes for card detection and such.

@AdrianSoundy
Copy link
Member

Ok i'll look at adding the native code required so i can test it
@josesimoes

@AdrianSoundy
Copy link
Member

AdrianSoundy commented Sep 9, 2021

Update:-
I added native code for ESP32 and in the process of testing. For other devices will just get NOT implemented exception for now.
Seems to work ok with 1bit & 4 Bit MMC interfaces. With SPI seems to be very selective on SDcard used.

Will look at doing a card Insert event as well but will need to know the GPIO used for that from managed code.

Still need to tidy things up and looking at making it possible to just load System.IO.Filesystem without Window.Devices.Storage.
Also noticed GetDirectories is not working but cards mount OK

There is an outstanding problem. You can't access the Internal partition as its now fixed to FAT32 where partition is Spiffs.
Windows.Devices.Storage used VFS to access both.

@josesimoes josesimoes changed the title Add external events and drivers. Add external events and drivers Sep 12, 2021
@AdrianSoundy
Copy link
Member

AdrianSoundy commented Sep 15, 2021

There is no way to specify a GPIO for detecting the SD card insertion unless its hard coded in the build.

Maybe we should create an instance of a SDCard class instead of a static class with constructors for MMC & SPI with an optional Inserted GPIO number. Then native code can fire event when card inserted. Also have an inserted property

@josesimoes @networkfusion

@networkfusion
Copy link
Member Author

@AdrianSoundy Sounds sensible to me... but would have to rely on @josesimoes for thoughts...

@josesimoes
Copy link
Member

There is already nanoframework/Home#808 which is kind of inline with this.

🆗 I'm quite all right with adding those constructors with those parameters. Those should be overloads to the parameterless one.

⚠️ And any changes (both C# and native) must not harm the current implementation for hard coded SD card detection (which ChibiOS has out of the box).

Changed from static class so it could be initialised with parameters
Then is can be used either by polling IsCardDetected state or waiting for event via StorageEventManager
@AdrianSoundy
Copy link
Member

@josesimoes @networkfusion
Can you have a look at latest change before a update native stuff.

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.

Looking good!

README.md Outdated Show resolved Hide resolved
System.IO.FileSystem/nanoFramework/SDCard.cs Outdated Show resolved Hide resolved
System.IO.FileSystem/nanoFramework/SDCard.cs Show resolved Hide resolved
System.IO.FileSystem/nanoFramework/SDCard.cs Outdated Show resolved Hide resolved
System.IO.FileSystem/nanoFramework/SDCard.cs Show resolved Hide resolved
System.IO.FileSystem/nanoFramework/SDCard.cs Outdated Show resolved Hide resolved
@AdrianSoundy
Copy link
Member

@josesimoes Seems to be failing with a error in powershell build script, can you have a look

@josesimoes
Copy link
Member

@AdrianSoundy all green now

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! I've missed this last request for a review. Apologies.

@josesimoes josesimoes merged commit fd439c6 into develop Oct 11, 2021
@josesimoes josesimoes deleted the develop-removable-events branch October 11, 2021 21:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants