WinRT Fixes and UWP Wrapper #1045

Open
wants to merge 5 commits into
from

Projects

None yet

3 participants

@donald-hanson

This is based on the google group discussion: https://groups.google.com/d/topic/openzwave/O74OL4BDrS0/discussion

There are 4 commits each meant to address something interrelated:

  1. add missing command class files and new http related files
  2. HttpClient needed to be updated to compile on WinRT as certain APIs were Win32 only
  3. WinRT SerialControllerImpl was not finding the Z-Stick Gen5 during testing
  4. UWP Wrapper project to expose OZW to C# on UWP platform
@Fishwaldo
Member

Question? Is it not possible to use the existing dotNet files for UWP? I'm not a huge fan of having to maintain another wrapper that will most likely get stale and out of date.... :(

@Fishwaldo Fishwaldo self-assigned this Nov 25, 2016
@Fishwaldo
Member

ping

@dotMorten
dotMorten commented Jan 10, 2017 edited

@Fishwaldo Not with the way you're exposing the .NET library.
If you want that, we need a C SDK with methods exported, and a pure C# managed library on top that calls these exports. We could then use the same .NET library, and just have two build targets for the native C-SDK. This is generally how you would create a cross-platform .NET library that has native dependencies.
I don't think you will get around the need for separate native implementations, since getting access to serial ports are quite different (unless we also abstract that out into a C SDK).

@donald-hanson has actually done a pretty good job at keeping this wrapper as slim as possible and re-using existing implementations internally. So all this really does is expose the same classes that .NET SDK does, but as WinRT ref classes instead. I was actually about to do the same work, when I released Donald beat me to it.

@dotMorten

Found a couple of building issues with the PR

+ <PropertyGroup Label="UserMacros" />
+ <PropertyGroup />
+ <PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Debug|Win32'">
+ <GenerateManifest>false</GenerateManifest>
@dotMorten
dotMorten Jan 10, 2017

I found I had to add this here to each configuration to get it building in all configurations (ie same addition to the 5 property groups below this one):

    <IntDir>$(PlatformTarget)\$(Configuration)\</IntDir>
    <OutDir>$(SolutionDir)$(PlatformTarget)\$(Configuration)\$(MSBuildProjectName)\</OutDir>

IntDir isn't strictly necessary, but then it matches OpenZWave project.

Without explicitly defining the outdir, Win32 is going into an odd folder in VS, and that target will fail to build, and I think it's good order to make sure these outdir matches the outdirs of the other project, in case VS ever changes defaults.

+# Visual Studio 14
+VisualStudioVersion = 14.0.25420.1
+MinimumVisualStudioVersion = 10.0.40219.1
+Project("{8BC9CEB8-8B4A-11D0-8D11-00A0C91BC942}") = "OpenZWaveUWP", "OpenZWaveUWP.vcxproj", "{46641B66-9271-4234-8E8B-1547DEF4BEBB}"
@dotMorten
dotMorten Jan 10, 2017

Inside here set a build dependency on the static library:

    ProjectSection(ProjectDependencies) = postProject
        {E830F9D6-8173-4B0B-9ECE-CAADFA531B54} = {E830F9D6-8173-4B0B-9ECE-CAADFA531B54}
    EndProjectSection
+(
+ String^ _configPath,
+ String^ _userPath,
+ String^ _commandLine
@dotMorten
dotMorten Jan 11, 2017

I would suggest hardcoding these, or at least provide a simpler overload. The nuget package would always deploy the configs in the same folder, the userPath would always be Windows::Storage::ApplicationData::Current::LocalFolder->Path.

@Fishwaldo
Member

As noted, my problem is with having "another" independent wrapper is it will bitrot. The .Net wrapper already is bitrotting and lags behind the main C library.

Additionally, pardon my ignorance of the windows platform, but when you say a C-SDK wouldn't the "uwp/src/" and "dotnet/src/" be mainly the same (apart form what you mention, the Serial Port code etc). I'm ok having that library #ifdef etc.

I've actually been tossing up splitting the existing "dotNet" wrapper into its own github repository and asking for a maintainer for it, as I don't really have the experience to maintain it myself. If you or @donald-hanson or anyone else is interested in taking that up, then you can run it anyway you see fit :)

@dotMorten

To be honest I had thought about forking it, as I don't find the .NET library very easy to use for a .NET developer. I think there's quite a lot that could be done to make it feel more like an API a .NET developer would use. I just don't know how much time I could dedicate. I'm already swamped in maintaining other IoT Libraries (the latest an Iotivity to ZWave wrapper hence why I'm in this thread).

However splitting it in two, might make bit-rot even worse? Would it be more helpful to create some issues here, and perhaps we can help getting it back up to speed issue by issue?

Wrt to the UWP and .NET duplicate code: I think we should be able to create some templates that allows us to re-use the code and just if-def the templates for each platform, so there's just one thing to maintain.

@dotMorten

Took a second look at @donald-hanson's awesome PR, and compared it to the .NET one: The code is almost identical, but with some key differences. I think it should be doable to make it a common code-base with if-def. That would at least resolve one of your concerns.

@dotMorten
dotMorten commented Jan 11, 2017 edited

@Fishwaldo Just another thought: If I were to take over the .NET portions of this project, would you be ok with changing the LGPL license of the .NET piece to Apache v2.0?

@donald-hanson And would you be ok with me forking with your PR and start building it up as a proper nuget package, and clean up some of it? (as well as re-releasing that bit under apache). Of course I'd gladly credit you and collaborate on it.

I'd also be ok with MIT license, but I can't get my self to do any significant work under such a restrictive license.

@dotMorten

@Fishwaldo Sorry for keep posting here like a maniac :) If we were to split it into a separate project, I assume you'll still maintain the WinRT project file for building the static library? @donald-hanson made some updates to the project file (added missing files) and a fix in HttpClient and SerialControllerImpl that I think are still critical to go into this repo.

@Fishwaldo Fishwaldo added a commit that referenced this pull request Feb 17, 2017
@dotMorten @Fishwaldo dotMorten + Fishwaldo Updated WinRT build to include newly added files (#1096)
* Updated WinRT build to include newly added files, and allowed more serial devices to be discovered.
These changes are cherry-picked from @donald-hanson's PR #1045, but excludes all the parts that adds a new WinRT Dynamic Library.

* Bug fix preventing crash when removing a driver
e44a0f3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment