search

sharpbrick / powered-up

.NET implementation of the LEGO PoweredUp Protocol
MIT License
94 stars 19 forks source link

Add Plugin Support to dynamically load BLE Adapters #171

Closed tthiery closed 3 years ago

tthiery commented 3 years ago

Closes #155 non-breaking

tthiery commented 3 years ago

@dkurok Can you have a look into this?

It is basically - for the generic purpose CLIs - a plugin infrastructure which loads the adapters from a sub folder using a configuration parameter (either using JSON or command line --BluetoothAdapter BlueGigaBLE --EnableTrace true).

Just watchout, this is still a draft. I have not written yet the steps to publish the WinRT/BlueGiga as plugins (it is basically a dotnet publish on WinRT/BlueGiga and then a copy into the CLI/Examples project directory) and package it into the poweredup tool.

dkurok commented 3 years ago

@tthiery I've looked over the changes and that looks promising and a good idea. I tried to get it running by building the BlueGiga-project and copy the output to a directory plugins\ble-BlueGigaBLE under the examples\SharpBrick.PoweredUp.Examples\bin\Debug\net5.0 - directory. The example-app finds the SharpBrick.PoweredUp.BlueGigaBLE.dll there, BUT in ServiceCollectionExtensionsForPlugins.cs, line 30 it throws in assembly.GetTypes(): System.TypeLoadException: Method 'Configure' in type 'SharpBrick.PoweredUp.BlueGigaBLE.BlueGigaAsPluginStartup' from assembly 'SharpBrick.PoweredUp.BlueGigaBLE, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null' does not have an implementation.

I looked some promising issues on the internet about this error, especially https://stackoverflow.com/questions/948785/typeloadexception-says-no-implementation-but-it-is-implemented But I cannot get it to work whatever order I build / copy / rebuild.

Did you get it running with WinRT ? If yes, what is your build-order / copy-order?

Another question: is the targetFramework "netstandard2.1" in the kernel-project (SharpBrick.PoweredUp) really needed anymore? Same for SharpBrick.PoweredUp.Mobile - there it is ONLY netstandard2.1. AFAIK "net5.0" would be enough; in .NET 5 there is no more ".NET standard x.y"

Did you try with a JSON-file instead of commandline-args? Can you checkin an example (not clear about the nesting of the options for e.g. the BlueGiga; couldn't really try because I'm not getting so far....

dkurok commented 3 years ago

Addon: Is it maybe a problem with VS Studio 2019 vs. VS Code? Can you tell me which main extension for this project you use in VS Code? Maybe it's better to switch also to VS Code for me to avoid problems related to the IDE?

tthiery commented 3 years ago

I run exclusively VS Code with the C# extensions. But that should not be the case of the editor.

What I did was that I run a dotnet publish of the WinRT code (so copying all the relevant stuff together and getting a production grade output of your project). And I copy that output in the plugin folder.

tthiery commented 3 years ago

Regards netstandard2.1: Yes, it is needed. I removed it but the Xamarin adapter needs it. Xamarin is not yet on net5.0 and will only arrive there with net6.0 => netstandard2.1 for them.

We will fix this in November.

tthiery commented 3 years ago

Regards the TypeLoad: That is most likely a c&p order thingy. build it completely. dotnet publish the bluegiga one and then copy it over. Yes WinRT works and I think I also tested BlueGigaBLE to the extend that it loaded.

tthiery commented 3 years ago

Regards poweredup.json: I have not tested. it should work like

{
  "EnableTrace": true,
  "BluetoothAdapter": "BlueGigaBLE",
  "COMPortName": "COM4",
  "TraceDebug": true
}

I have not stacked them (yet) because I do not know the behavior of the command line parsing in case it is nested.

tthiery commented 3 years ago

@dkurok I figured out your "not implemented" problem (and also the poweredup.json problem).

The plugin copied its own set of dlls (e.g. folder "Host" Microsoft.Extensions.Logging with IConfiguration and plugin folder "BlueGigaBLE" Microsoft.Extensions.Logging with IConfiguration). And the core lib was already not included in the plugin by provided by the host, the two IConfiguration did not longer match => Interface was not implemented by Type.

I could fix that by deleting files from the plugin.

However, then System.IO.Ports became a problem. The assembly resolver was not loading the right version anymore (the windows one). Working on it. And also fixed. Worked after all other assemblies were conflict free and the regular .deps.json file worked again.

tthiery commented 3 years ago

I am currently not in favor of this solution. The packaging effort of the plugins into the various other projects makes it incredible hard to understand for a beginner.

I work on an alternative, but the user-friendliness vs. maintainer effort is a nightmare

tthiery commented 3 years ago

I have decided to close this Pull Request due to the maintenance burden it implies. I will create a simpler, less automatic approach in #172.