OpenZWave / openzwave-dotnet-uwp

An Open-ZWave wrapper for use with .NET or UWP apps
Apache License 2.0
45 stars 27 forks source link

.netstandard #10

Closed grahamehorner closed 7 years ago

grahamehorner commented 7 years ago

Please consider targeting the .netstandard so the library may be used across the .net framework and .net core platforms

dotMorten commented 7 years ago

This is not a .NET library. It's a c++/cli for use with .NET and C++/CX for use in UWP. To make it a. NET std library it would have to be a .NET p/invoke wrapper around the native C library which wouldn't give you any extra platforms anyway.

dotMorten commented 7 years ago

Because this would be a complete rewrite and not built on top of OpenZWave and I'm not sure what the big value add would be, I'm closing this but feel free to comment more

grahamehorner commented 7 years ago

IMHO if the OpenZWave library/api was exposed is a similar manor to that of libuv and subsequently the .NET wrapper used was similar to Kestrel; you would have a truly portable OpenZWave .NET wrapper that could be used on many more platforms that windows and/or UWP including headless servers linux/windows nano server + iOS etc.

dotMorten commented 7 years ago

@grahamehorner That's somewhat true, and something I've actually done for other libraries several times. However, the wrapper would never support more than what the underlying OpenZWave library supports, which is Linux, MacOS and Windows (I don't really get how iOS would work anyway? How are you going to plug that USB stick into it?)

So that just leaves adding Linux and MacOS. As mentioned, it would be a complete rewrite of the entire API to support such a thing by using p/invoke into a C-API instead of writing directly against the C++ core library. That's a major undertaking, and quite frankly not something I have the time nor means to do. I'm leaning towards that would have to be a separate project driven by those who really need Mac and Linux for .NET Core. I'm curious how many people are actually asking for this and wouldn't just use the native library straight up, so hopefully more people would chime in here.

darrenhull commented 6 years ago

It’s not a complete rewrite, it’s just the c++ manager that needs converting to c. We have done this at my work already. As it allows us to use a dotnetcore solution on windows and Linux. If there is a desire for it I will see if we can push it back into the community.

grahamehorner commented 6 years ago

I've been looking into a number of protocols that are used in HA and doing a some RnD on using Azure IoT Edge of the Raspberry PI as a Edge gateway; also been investing in TheThingsNetwork/LoraWan as a possibility for remote location where GPRS/3G/4G are weak; I personally love to see this happen as I feel it could be a great feature of an opensource IoT Edge gateway :D and would be happy to help

dotMorten commented 6 years ago

The conversion to C is only a small part of the battle. The next part would be to create an entire set of .NET classes that does the interop to the C api layer, not to mention life cycle management now has to be done more carefully. This is a huge amount of work with a lot of risk, "just" to add support for Linux. I'm curious if this is just a nice-to-have vs an actual need.

If you want .NET Standard support, you could wrap the WPF and UWP APIs in a .NET compatible API surface, and use bait'n'switch in the nuget package. That would be a significant less amount of work just to .NET Standardize the API surface.

grahamehorner commented 6 years ago

I'm happy to help in both areas; I'm a professional developer with over +20years experience and code in C, C++ .NET C# F# and more; with regards to .NET Standard I look to target .NET Core .NET Standard 2.0 and not the .NET Standard Full framework which wrapping the WPF/UWP would need.

darrenhull commented 6 years ago

I think your overstating how much work it is, it wasn’t that much work at all. We already use it in a dotnet standard library hosted in a docket IoT solution.

darrenhull commented 6 years ago

I would be keen to push our solution into this repo so that people can pull it apart and make it better.

grahamehorner commented 6 years ago

@darrenhull is the docker image you've mentioned on docker hub ?

darrenhull commented 6 years ago

No unfortunately not as it is in a private repo.

grahamehorner commented 6 years ago

@dotMorten @darrenhull I’d be happy to fork and start this work in the open if @darrenhull can’t make public the work he and his employer has done in this area? with the idea that this could then be merged back as a pull request. Any changes of cause shouldn’t/wouldn’t break existing interfaces or implementations and have unit/integration tests

grahamehorner commented 6 years ago

FYI https://github.com/Azure/iot-edge https://docs.microsoft.com/en-us/azure/iot-hub/iot-hub-devguide-sdks

dotMorten commented 6 years ago

I think your overstating how much work it is, it wasn’t that much work at all.

Here's where I'm coming from: Replacing the current pure C++ library, with an interop-based C and .NET library would mean no code would be left in this repo. It's a complete rewrite. I think that's totally fair to say that is a lot of work, with a lot of risk, with a very small value add.

I'm very well skilled in writing C interops. I've made several cross platforms ones, and the main part of my day to day job is built around it, so I do know what this entails.

Considering the suggestion involves a completely different brand new implementation, this isn't a fork. It's a new repo. I'm not saying it isn't a cool suggestion, but I think it's better off being in its own repo. If it becomes a solid product, I'm sure it could be move into the openzwave repos. That's how this repo came about as well.

darrenhull commented 6 years ago

That does sound like a lot of work. We just rewrote the current c++ dotnet wrapper and created a dot net standard interop layer. Like I said I will see if I can create a PR of our implementation.”.

grahamehorner commented 6 years ago

@dotMorten sorry if it seemed as if I was calling into question your experience; that wasn't intended or what I was thinking, I was trying to outline my background and experience and my willingness to help move this forward as IMHO I feel a portal Z-Wave API written using ansi C with a .NET standard library would not only be cool but a very popular implementation for IoT gateways and HA etc.

I'll take on=board your comments with regards to forking this implementation and start a new project; and would welcome any feedback and/or assistance that you or @darrenhull or anyone would like to offer at any point should you wish to contribute.

FYI, I'll be looking into possibly using named pipes to communicate between a z-wave layer and/or .net standard library

dotMorten commented 6 years ago

Don't get me wrong: Had I started fresh this might also be the approach I would have taken. My issues doing it at this point is:

  1. It would yet again introduce breaking changes.
  2. There's no .NET Standard for serial port, so you need platform specifics anyway
  3. It's currently extremely easy to maintain, as new members can simply be done by adding a member to the class - no need to wrap in a C-API and then import in C# (had OpenZWave already been a C API it would have been even more obvious to go that route).
  4. Value add for .NET Standard is somewhat limited (and it would be quicker to just wrap the existing library with a .NET Standard layer on top).
  5. I'm not getting the sense Linux support is really such a big ask from anyone.
  6. It's an entirely new approach and library, and as such IMHO should be a separate repo/library - if it's successful, I don't see why this repo couldn't get deprecated.

I'd really encourage building this library and I'd be happy to help with pointers. I've learned quite a lot of tricks over the years making .NET/C-interop work cross-platform (windows, uwp, android, ios, tizen, Linux, macos).