angularsen / UnitsNet

Makes life working with units of measurement just a little bit better.
https://www.nuget.org/packages/UnitsNet/
MIT No Attribution
2.2k stars 379 forks source link

UnitsNet types are not Windows Runtime Component Compatible #150

Closed jbienzms closed 8 years ago

jbienzms commented 8 years ago

I would like to use UnitsNet in a large Windows IoT Core Devices project I manage called Microsoft.IoT.Devices. However, when I attempt to use the types defined in UnitsNet as return values in my library I get the following error:

Severity Code Description Project File Line Suppression State Error WME1032 Method 'Microsoft.IoT.DeviceCore.Sensors.ITemperatureReading.Temperature.get()' returns 'UnitsNet.Temperature', which is not a valid Windows Runtime type. Methods exposed to Windows Runtime must return only Windows Runtime types. Microsoft.IoT.DeviceCore C:\Users\jbienz\Code\Microsoft\IoTDevices\Lib\Microsoft.IoT.DeviceCore\Sensors\Temperature\TemperatureReading.cs 23 Active

This is due to the type restrictions on WinRT components.

Obviously UnitsNet was written as a Class Library and not a WinRT component. And obviously refactoring the library into a WinRT component may require that some features be left off. However, I still feel this would be incredibly valuable to have, especially in the IoT space.

If you would like to discuss this more please feel free to reach out to me. com dot microsoft at jbienz, or via github.

angularsen commented 8 years ago

Hi, thanks for reaching out.

I'm not familiar with developing towards WinRT, but from this SO answer I understand that UWP C# apps can interact with Units.NET just fine, but the CLR must project the managed types to Windows Runtime types when consumed by Windows Runtime Components.

I read your link and see there are some constraints to the types used. Below are some initial thoughts, please correct me if I am wrong.

The fields, parameters, and return values of all the public types and members in your component must be Windows Runtime types.

So basically, stick to Int32, Double, DateTimeOffset and a small set of .NET types. We should be near this, I think.

Public structures can't have any members other than public fields, and those fields must be value types or strings.

All unit types are public struct today, and they have both properties and methods. I assume this must be refactored to class instead? Should be doable.

Public classes must be sealed

Not a problem.

A public class or interface cannot:

  • Be generic.

I believe we are good here.

  • Implement an interface that is not a Windows Runtime interface. (However, you can create your own Windows Runtime interfaces and implement them.)

We use very few interfaces today, I think we are good here.

  • Derive from types that are not in the Windows Runtime, such as System.Exception and System.EventArgs.

We have a few exception types, but other than that we should be good.

I'll try to set up an UWP app with a Windows Runtime Component and reproduce the behavior you describe, and try to get a better picture of what refactoring is required to be compatible. If it is feasible, then I'd be happy to put in some effort to get there. The strategy might involve modifying the code generator scripts to output two sets of source code; .NET and WinRT.

I'll get back to you when I know a bit more, likely by tomorrow or the day after.

jbienzms commented 8 years ago

Thank you so much for getting back so quickly anjdreas. You are very very much on the right track in your thinking above. Honestly, here's the easiest way to see what would need to be changed:

  1. Open your class library project in Visual Studio 2015
  2. Right-click on the project and open Properties
  3. On the Library tab (first tab), change the 'Output type' dropdown from 'Class Library' to 'Windows Runtime Component'
  4. Close Properties
  5. Rebuild the project

The compiler will tell you each and every item that isn't compatible and actually gives fairly helpful links and messages about making the conversion.

The main value in performing this work is that it allows your library to be projected natively into any language supported by the Windows Runtime. Therefore your library can be used not only by C# and VB, but also by C++ and JavaScript. This is possible through the magic of language projection, but language projection comes at the cost of the restrictions mentioned in that link.

In addition to your library being able to be used by those languages, it also allows your library to be used as part of other libraries that target those languages. In my case, my library is about making it easy to access sensors and devices on Windows IoT Core. I could have chosen to implement my library as a Class Library instead of a Windows Runtime Component, but I know that a lot of IoT developers use C++ and I know that a lot of students find it interesting that they can write full fledged apps on IoT devices using JavaScript. If I wrote my library as a Class Library, it couldn't be used by those folks. Porting to a Runtime Component also allows your library to be used in those languages on new devices like XBOX One and HoloLens.

To get me by in the meantime, I've added a Temperature class to my own library that's based very much on your implementation and I gave you credit in the source. My implementation is nowhere near as robust as yours but it's a start and if your library ends up being Runtime Component compatible I'd like to yank out my implementation and just take a dependency on UnitsNet. (Thank you for publishing your code under a very non-restrictive license, BTW. I wouldn't have been able to do that otherwise.)

As you investigate making the move to a Runtime Component you may or may not find features of your library that won't work under the Runtime Component restrictions. If everything moves over easily, AWESOME. Then it's just about changing the way the library is compiled and changing the binary that NuGet deploys for UWP apps. However, if you find features that have to be removed I recommend using conditional compile statement and creating a new NuGet package called something like UnitsNet.UWP (Universal Windows Platform). You may want to do this anyway, for example, if you want to keep your core types as structs instead of classes on non-UWP platforms.

If you have any questions about this process whatsoever please don't hesitate to reach out. I've already endured the pain - err, I mean, enjoyed the learning exercise - of porting several Class Libraries to Windows Runtime Components. I'm more than happy to share knowledge or help with translation.

Thank you again for such a quick response and for looking into it!

Jared

angularsen commented 8 years ago

image

I think this should be doable. I got a working prototype with a Windows Runtime Component where I copied in Temperature and its dependencies, and modified them until they compiled.

Branch here: https://github.com/anjdreas/UnitsNet/commits/uwp

The features we lose are:

angularsen commented 8 years ago

I'll take a look at this tomorrow again, getting late here now. It's possible we can do with some conditional compile statements, but there were enough changes to get this prototype working that I'm worried it will be a bit of a mess to keep both variants in the same files. We will see.

Either way, conditional compile or separate code for WinRT and .NET, I think I should be able to pull this off over a couple of evenings.

Thank you for your assistance and input, it is much appreciated.

jbienzms commented 8 years ago

I'd love to help if you need it. I would never try to tell anyone how to maintain their own library, but I would love to see this done with conditional compile statements. The main reason is all too often I've seen wonderful libraries like yours get forked for one odd reason or another and the forks tend to fall behind when new features get added to the main branch. You appear to be super active on your project, which is amazing, and I'd hate to see the UWP fork fall behind.

If it would help for me to jump in and do some of the work, I'm happy to contribute some time. You could just put everything in the UWP fork and we could divide the work and keep plugging away until we get something workable that we can merge back into the main branch.

Let me know what you think after you've had some time to tinker with it. You've created an awesome library.

angularsen commented 8 years ago

Thanks, I will probably pick you up on that offer. I propose I scramble together an initial draft with modifying generator scripts and using conditional compile statements, then you can review that and we take it from there.

To be clear, my intention was never to fork this out in such a way that it would fall behind. It will be part of the main library and main repo, hopefully even the same nuget, but seeing we are already generating most of the code using powershell scripts it may or may not be better to simply generate a second set of source code targeted at WinRT - purely as an alternative to conditional compile statements.

Just a heads up, we are awaiting a newborn anytime today or in the next few days, so I may be unavailable at least a couple of days when that happens.

jbienzms commented 8 years ago

First and foremost, congratulations!!! I'm quite familiar with that time of life, so no worries at all and enjoy it!

I also didn't realize how much code is being generated. If that's the case, conditional compiles aren't nearly as important for keeping things in sync. I'll totally defer to you on which approach you think makes the most sense, and I'll help however I can.

The only part I think we need to be careful with is putting it all in the same NuGet package. At least at the moment I'm not thinking that's a good idea. To your point above, this change does require dropping some valuable features from the library like operator overloads and custom exceptions. If we crate a new NuGet package (something like UnitsNet.WindowsRuntime) then Windows 10 developers who write their apps in C# and VB still have the option to use the more fully featured version of your library. However if we add the Uap10.0 folder to your existing NuGet package, that folder will have a higher precedence over all other versions in your package in Windows 10 apps. And that will force the usage of the Runtime Component version on Windows 10, even if the more fully featured version could be used. I don't think we want to force this change when it's still completely valid to use the Portable / Net40 version in Windows 10 apps that are written with C# and VB.

What do you think?

eriove commented 8 years ago

Nice to see more people interested in the library.

While I have no intermediate use for this feature I like libraries supporting as many platforms as possible (and I do have some small Windows IoT projects on the side). Hopefully most of the functionality could be kept.

As @anjdreas has suggested I would suggest to generate another set of code for WinRT but I would also suggest to completely share the "handwritten" code and if necessary use conditional statements in these files. That way we'll get the best of both worlds...

angularsen commented 8 years ago

So I'm back and all is well with the new family member.

I took a stab at this and there is now a WIP nuget out based on the uwp branch.

Rambling thoughts

Code refactoring went pretty much as expected. I did stick with conditional compile statements, and I think I'm fine with how it turned out. I mostly wound up internal-izing methods with generic params or incompatible param types, and added some alternative public members that now are available in both main lib and uap lib.

I first attempted a single nuget with just a new uap10.0 target, but as you pointed out @jbienzms, the test app pulled in the uap target instead of the full .NET lib, and for c# apps that is probably not what you want. So I added a new UnitsNet.Uap nuget for the sole purpose of exposing a stripped down Units.NET library to those developing Javascript or C++ UWP apps, or developing their own WinRT Components to be consumed by any platform.

I must say, I did struggle a bit with the new nuget system. Apparently packages.config files are on the way out, replaced by project.json files. However, only PCL, UWP and ASP.Net Core can use this at the moment, so the .NET 3.5 project gave me all sorts of trouble. I didn't find any open source project to use as reference, so I rolled my own the best I could. I'm sure it can use some improvements, but basically I let the old projects stay on packages.config files, and only the new Uap project has its own personal UnitsNet.Uap.project.json file.

Request for comments

  1. Should nuget and project be named UnitsNet.Uap or UnitsNet.Uwp? To my knowledge they refer to the exact same thing, but UWP is the most recent naming. Nuget still uses uap and uap10.0, which adds to the confusion.
  2. How can we best ensure main lib and uwp lib are synchronized? Currently I need to edit both .nuspec files before releasing, which is a minor nuisance.
  3. Is description/summary in UnitsNet.Uap.nuspec ok?
  4. Should we add a new solution file for UWP?
    1. Building the solution became a lot slower when it's also building the UWP test app and/or UWP lib. Apparently this is a known issue with UWP, probably related to the projection stuff.
    2. I assume users will get errors when opening the solution without having X and Y plugins installed. At least I had to install some stuff to get started.
    3. With multiple solutions it's hard to ensure you are not breaking anything when editing shared code. However, the main contribution is adding new units and that is purely JSON + generated code. Also, the build server will help catch this on PRs.
eriove commented 8 years ago

@anjdreas Great work!

  1. I would go with Uap if NuGet uses it. Branding is much more likely to change than tags in config files.
  2. Could you set one and let a build script add the other? Seems like much work for a small problem though. Something similar to this could be used.
  3. Seems fine, Can't say I've spent much time reading NuGet package description though.
  4. I've seen many other projects with solutions on the form ProjectName.All.sln, ProjectName.Silverlight.sln etc. This seems quite established.

    i. I didn't notice a difference in build speed at all. Restoring packages takes 5-10 seconds and building 1-2. Probably very dependent on computer. Intellisense gets really confused with the #defines though. ii. This is a very good argument for multiple solutions. iii. Since developers not using Windows 10 can't compile the UWP apps anyway (Windows 10 and VS2015 listed as requirement here) adding multiple solutions makes it less awkward for them at least. The build server still has to do the actual verification that the code compiles.

angularsen commented 8 years ago

Thanks, I'll split into a separate solution then.

@jbienzms Have you had time to look over this yet? I still don't know if it actually works in an JS and C++ app.

jbienzms commented 8 years ago

Well shoot, I was replying to the e-mail bot last week and apparently it didn't actually post my replies here. Let me first start by posting those messages that didn't make it:

Monday, March 14th @ 3:28 PM

Driving right now so can't do a full reply, but I recommend against UWP and instead suggest UnitsNet.WRC or UnitsNet.RuntimeComponent. Because UWP will confuse Win10 devs Into thinking that’s the one they should use.

I also recommend saying something at the very beginning of the NuGet package description that it is a slimmed down version for compatibility with all UWP languages and runtime components. And that developers targeting ONLY C# or VB are still recommend to use the other package.

I will respond more soon. Awesome work and congratulations on the new addition to the family!!!

Monday, March 14th @ 3:34 PM

P.S. If you disagree and want to use UWP, that’s ok too. Just be really clear in the description. Don't use UAP. Yes, NuGet still uses it but they shouldn’t. They keep it in there for legacy compatibility. Technically UAP is Universal App Platform and UWP is Universal Windows Platform. Technically UAP was Windows 8.0 & 8.1, where UWP is Windows 10 and later.

Now, on to your more recent questions:

Building the solution became a lot slower when it's also building the UWP test app and/or UWP lib. Apparently this is a known issue with UWP, probably related to the projection stuff.

Actually, this is probably due to everything getting compiled for .Net Native. There's an awesome article on what .Net Native is and what it means for your apps and libraries here. Though technically this shouldn't be happening for your library, just for any test applications or harnasses that use the library. Libraries can still be distributed as Any CPU, though UWP applications must always be compiled to a CPU target (x86, ARM, etc.)

Should we add a new solution file for UWP?

I would say probably yes. Mainly because some of your developers may not have the latest Visual Studio and may not have the tools for Windows Store development installed. In those cases they will see errors. Since you've done all the hard work to do conditional compiling where it makes sense, obviously those two solutions can share the same files. The only downside to this approach is that a community contributor could fix a bug or add a feature in a pull request that compiles in the NET solution but doesn't compile in the UWP solution. Meaning, as you accept pull requests you'll want to do your own compile against both solutions to make sure they work or refactor them.

Have you had time to look over this yet? I still don't know if it actually works in an JS and C++ app.

If it compiles and can be referenced from another UWP component, it should work in all languages. The issues you've already dealt with (like default overloads, etc.) are what allow the code projection to work. We could build some WinJS test harnasses, but honestly I think that's not necessary.

I'm a bit under water coming back from being out of the office last week but let me do a mini code review real quick.

Vector3.cs Yup, that looks good. Though I think you can still support parameterized constructors on a struct as long as you include a default empty constructor.

UnitsNetException.cs Yup, it's probably easiest to just make it internal. It can still be thrown even though it's internal.

UnitClasses/Temperature.g.cs Looks correct. Unfortunately the DefaultOverload attribute doesn't work on constructors. This could be refactored to take a number and a units of measure enum and combine them all into one but that would change the signature of your existing library and you already have the static From method that does the same thing so I don't think that refactoring would make sense.

Conditionally compiling out the nullable types is all correct. Unfortunately.

I see that a lot of your conditional compile statements are there because of changing the access modifier (changing public to internal). I wish there was a shorthand for this but I don't see one. There is no such thing as creating an alias for an access modifier.

For all of the ToString methods you might consider using a type alias at the top of the class. For example:

#if UWP
using Culture = System.String;
#else
using Culture = CultureInfo;
#endif

Then you could write a single method body like this:

public string ToString(TemperatureUnit unit, Culture culture)

Of course you would still have to use conditionally compiled code within the body because the code paths are pretty different, but at least there would only be one method body instead of duplicating the whole blocks of code just to change the signature. The one thing this doesn't account for though is your use of [CanBeNull] when Culture is a string. That appears to be a ReSharper thing so I don't know if that's important.

Please let me know if there's a particular piece of code you'd like me to review. So far it looks great. And when you have a theoretical NuGet package published let me know that too and I'll reference it in my IoT library to see how it works out. When you're ready to go live, I can push an update on my library to start taking a dependency on yours.

Thanks for all the hard work man! I'm super impressed!!!

angularsen commented 8 years ago

1. Nuget name: UnitsNet.Uwp

I did a quick search on packages on nuget.org: Search for winrt returned 706 packages Search for uwp returned 352 packages Search for uap returned 64 packages Search for RuntimeComponent returned 9 packages Search for wrc returned 0 packages

Search for tag:uwp returned 294 packages. These are the first ones with any relevant term in their ID. SQLite.Core.UAP Sqlite-Winrt.uwp DotBlog.MvvmTools.AutoINPC.UWP HAPUWP GHIElectronics.UWP.GadgeteerCore LumiaImagingSDK.UWP LumiaImagingSDK.Javascript UwpDesktop Bazam.Uwp WeatherNet.uwp Common.Logging.WinRT UWPDataTrigger

By browsing a bit, very few recent packages seem to follow the winrt naming. UAP has few packages and the two last terms even fewer, which makes UWP my favorite. I wish the nuget team made a clear guidance on this, as the variation in package namings are way out there.

2. Modified nuget description

I modified the description/summary to be even more clear about being a stripped down version. Does this work?

<title>Units.NET - Windows Runtime Component</title>
<description>Stripped down version for Windows Runtime Components in UWP apps, in order to be compatible with JavaScript and C++ UWP apps. For a C# UWP app, consider the full library instead.</description>
<summary>Stripped down version for Windows Runtime Components in UWP apps.</summary>

3. Added solution UnitsNet.Uwp.sln

Moved UWP projects out of main solution.

4. No UWP specific tests

I do note we have no tests explicitly for UWP, but I'm not sure if it's worth a lot of effort. The shared code is quite tested and I don't see a lot of potential for bugs in the conditional compile statements, knock on wood, so unless you have anything in particular in mind I vote we leave it without tests for now.

5. Building UWP nuget with AnyCPU

You did mention this, but I just wanted to make sure that AnyCPU is the way to go? I've seen nugets named like MyLib.Uwp-arm and MyLib.Uwp-x64. What purpose does that serve?

6. Type alias Culture for string/IFormatProvider params

Thanks for the tip, that was clever and saved me a lot of duplicate code.

7. Nuget is out

https://www.nuget.org/packages/UnitsNet.Uwp/3.30.0-alpha2

8. Test on a JS or C++ UWP app

It would be really good if you could test the nuget on a JS or C++ app to make sure it works as advertised, but if you are confident about that part working then I'm fine with that.

9. PR is ready to merge

https://github.com/anjdreas/UnitsNet/pull/152

I'd like some feedback on this last work and the numbered sections, before merging in the PR.

jbienzms commented 8 years ago

I did a quick search on packages on nuget.org

Don't forget that all of those naming conventions were valid at different times. WinRT was Windows 8.0, UAP was Windows 8.1 and Windows Phone 8.1 and UWP is Windows 10 and Windows Mobile 10. Just looking at the total number of packages that use a particular suffix is not going to tell you which suffix is the right suffix for your project.

which makes UWP my favorite

You absolutely can use the UWP suffix if you want. Just be aware that when a Windows 10 developer searches NuGet for packages they will see one entry for UnitsNet and one entry for UnitsNet.UWP. Windows 10 developers have sort of been "trained" that when they see UWP they see "that's the package for me". In your case, though, that's actually probably NOT the package they want the majority of the time. The only time a developer wants the this version of the package is if they're writing a Windows Runtime Component to target all possible languages. The average UWP developer is building an app, not a library. So the average UWP developer is likely to pick the wrong package if they see this one listed as "UWP". That's specifically why I'm recommending using a suffix that is uncommon. We don't want the average developer to select this version. Your main version offers many more features and is compatible with most UWP apps. We want them to pick your main version by default.

Again, if you feel strongly about using the UWP suffix, please consider making sure the very first line of the package description starts with something like "Most UWP developers should NOT use this version and instead use the NuGet package called UnitsNet. This version of UnitsNet is compiled as a Runtime Component to support all UWP languages and other runtime components. It has reduced functionality compared to the main UnitsNet package."

I modified the description/summary [...] Does this work?

Yes. Though "stripped down" does sound kinda bad now that I read it back (I know I suggested something like that previously). That's why I suggested the text above. But whatever you choose, you're on the right track. The primary concern is letting them know that if they're picking this version over the other one they should be doing it for a specific reason.

so unless you have anything in particular in mind I vote we leave it without tests for now

Nope, I'm fine with that. Your other version is well tested and this version is mostly just hiding some things. You could possibly write some unit tests for string conversion because that path is different, if you wanted to, but all your other test cases seem well covered.

Nuget is out

Excellent. Let me know if any of my notes above change your mind or if you decide to stick with the UWP suffix. When you've settled on the final naming convention for the package I'll update my library to consume it.

Test on a JS or C++ UWP app

I can do that when we have the final package naming convention nailed down. I'll build a sample app off of that package out of NuGet.

I'd like some feedback on this last work

Sure. Anything in particular? Looking at the Temperature class I see that a couple overloads of ParseUnit use CultureInfo explicitly (either (CultureInfo)null or new CultureUnfo(cultureName)). I just wanted to make sure that was the right approach now that you're using the alias trick to switch between CultureInfo and string. It looks like the main implementation of ParseUnit takes an IFormatProvider. Honestly I haven't done anything much with cultured string parsing so this may be the exact right way of doing it. I'm just pointing it out mainly as a note for you to to double check that's still the right way with the alias. It very well may be.

Let me know if there are any other areas or things you'd like me to review specifically. I'm more than happy to help.

jbienzms commented 8 years ago

Whoops, I missed one:

Building UWP nuget with AnyCPU

Yes. Your NuGet library should be built as Any CPU. Only applications and C++ libraries must be processor compiled. What actually happens with your code (believe it or not) is it gets merged into the application binary at compile time using the same processor architecture that the application is targeting. Fascinating stuff. That link I provided above on .Net Native explains in more detail if you're interested.

angularsen commented 8 years ago

Thanks for the detailed feedback.

1. Nuget naming

Your point is well taken and I agree it might be good to distance the nuget from the UWP naming. I then propose UnitsNet.WindowsRuntimeComponent.

2. Nuget description

Assuming we go with the naming in point 1, here is something in-between our two suggestions:

    <title>Units.NET - Windows Runtime Component</title>
    <description>For C#/VB Universal Windows code (UWP), use UnitsNet. This is a Windows Runtime Component with reduced functionality to support all UWP languages (JavaScript and C++) and other runtime components.</description>
    <summary>For C#/VB Universal Windows code (UWP), use UnitsNet.</summary>
3. Type aliasing

Thanks for pointing that out, I did double check those overloads and found a couple ones that could be improved to use Culture alias more. The main exceptions are ParseUnit() methods and UnitSystem constructor methods, which complicated things by being public and also used internally. That is why the (IFormatProvider)null needs to be there.

4. Tests

These are definitely needed. I created a JS UWP app to test this thing and right off the bat I'm hitting bugs due to ToString() parameter being string instead of CultureInfo. I've added some of the existing tests that did not require any modifications, mainly Parse() and ToString(). These are also the ones with the most conditional compile statements in them, so that was good to add.

5. Can't install nuget on JS app

I got to wrap up for today, but I hit a wall trying to add the nuget to the JS app. Adding the project reference works well, but not the nuget. Not sure what I'm doing wrong. Any ideas? I've pushed the latest bits to the branch. Also here is the latest nuget: https://www.nuget.org/packages/UnitsNet.WindowsRuntimeComponent

I'm leaving for easter holidays for a couple of days, so I will probably not be very active before Saturday or so.

jbienzms commented 8 years ago

Nuget description

Beautiful! Only a very minor suggestion: instead of (JavaScript and C++) perhaps (including JavaScript and C++)? When I read it I just felt it was saying only those languages were supported. Who knows if other languages will be supported in the future and of course C# and VB are already other options today.

Type aliasing

Understood. Thank you for re-checking just to be safe.

Tests - right off the bat I'm hitting bugs due to ToString() parameter being string instead of CultureInfo

Uh oh. That's honestly kind of unexpected. Can you share what your test code is, what the expected result was and the actual result? I'll see if I can explain why, make suggestions or dig into it more.

Can't install nuget on JS app

First and foremost, make sure you've selected a CPU architecture from the solution dropdown. I've noticed NuGet can get wonky if the application project type is set to Any CPU.

With x86 selected, when I try to reference your NuGet package I get the following error in the Error window:

Could not install package 'UnitsNet.WindowsRuntimeComponent 3.30.0-alpha3'. You are trying to install this package into a project that targets 'Windows,Version=v0.0', but the package does not contain any assembly references or content files that are compatible with that framework. For more information, contact the package author.

I believe the problem is somewhere in your .nuspec file. For example, it's worth noting that your .nuspec lists the following dependencies:

<dependencies>
    <group targetFramework=".NETPortable0.0-sl4+wp71+win8" />
    <group targetFramework=".NETPortable0.0-net40+win8+wp8+sl5" />
    <group targetFramework=".NETPortable0.0-net45+win8+wp8" />
    <group targetFramework="WindowsPhone8.0" />
    <group targetFramework="Windows8.0" />
    <group targetFramework="Windows8.1" />
    <group targetFramework="Unsupported0.0" />
    <group targetFramework=".NETPortable0.0-win81+wpa81" />
</dependencies>

In the .nuspec for my Runtime Component package, the only other dependencies listed are other Runtime Component packages I depend on:

<dependencies>
    <dependency id="Microsoft.IoT.DeviceCore" version="1.0.3" />
    <dependency id="Microsoft.IoT.DeviceHelpers" version="1.0.3" />
</dependencies>

I don't have any group or targetFramework elements but I could be totally doing it wrong. At the very least I think you at least need to add a targetFramework for uap10.0 to your dependency list.

Here's a good article on NuGet 3.1 and UWP support:

http://blog.nuget.org/20150729/Introducing-nuget-uwp.html

That's where I found that targetFramework string.

The only other difference between our nuspec files is you have a language tag and I don't. But I doubt that matters.

angularsen commented 8 years ago

including JavaScript and C++

Agreed.

Can you share what your test code is, what the expected result was and the actual result?

The bug in this case was trivial and my bad, but it was only triggered in the UWP code path and underlined the importance of having tests for both code paths in general. The tests I added should cover ToString and ParseUnit to some extent, but conversions and things like no decimal type are still untested in UWP code paths. My point is just that I proved myself wrong in assuming it was fairly safe to not test the UWP code, and it's probably worth adding a few more tests towards UWP code paths to improve coverage.

I believe the problem is somewhere in your .nuspec file. For example, it's worth noting that your .nuspec lists the following dependencies:

This section was actually just an experiment, based on the GoogleAnalyticsSDK nuget that worked. It did not help. Originally I did not have any dependencies at all, which I believe is correct for this lib.

angularsen commented 8 years ago

With x86 selected, when I try to reference your NuGet package I get the following error in the Error window:

Yes this is the error I got as well. Haven't had time to research it yet, but I will read your link. Hopefully this is just a matter of tweaking the nuspec.

angularsen commented 8 years ago

I think I got it all working now. I removed the <dependencies> section and restarted Visual Studio. Pretty sure I didn't touch anything else, and I'm also pretty sure I tested the exact same .nuspec structure earlier. Maybe I was stuck with some old nuget cache, not sure. Tried cleaning that too.

Anyway, there are now two UWP test apps; C# and JavaScript. Both can successfully install the nuget and they demonstrate some simple conversions and ToString() calls to output the results to a label. I tried adding a C++ app too, but got some errors trying to create the project.

I've edited the .nuspec as discussed, so I think this is getting ready to merge and publish to stable nuget now. Thoughts?

https://www.nuget.org/packages/UnitsNet.WindowsRuntimeComponent https://github.com/anjdreas/UnitsNet/commits/uwp

Notes: When installing nuget in JS app, you have to manually set <SpecificVersion> from True to False in the .jsproj file. You will get a build warning and a runtime exception that it can't find the UnitsNet type if you do not change this. I see the exact same behavior with the GoogleAnalyticsSDK nuget, so I currently assume this is a bug in the nuget package manager extension. I could not find an issue on this though, so I created an issue here.

jbienzms commented 8 years ago

I honestly think having to set SpecificVersion isn't right... Especially since you have to manually edit the .jsproj file. I tried adding my own WRC component to a WinJS file and I had a different problem. I got no error or warning symbol, but mine never showed up as a reference! I'm still looking into it. And I see you've got some traction on your issue. That's awesome. Let me know if it goes stale.

angularsen commented 8 years ago

Just an update, according to discussion in https://github.com/NuGet/Home/issues/2406 the JavaScript team is aware of the problem and working on a fix.

They also point out a workaround using project.json instead of packages.config, as detailed here.

jbienzms commented 8 years ago

Yes, I've been having lots and lots of discussions with them behind the scenes. Seems to be an issue with NuGet and the way WinJS projects use packages.config instead of project.json. I think we're fine to go ahead with publishing our updates. I don't think there's anything we can do on our end to help or work around it.

I have this scheduled on my end to take a dependency on your library probably early next week. This week I have a commitment due for a video on the Maker Show and I also received my HoloLens on Friday so I've been busy with creating some communities around that.

It looks like the WindowsRuntimeComponent version of UnitsNet is still pre-release on NuGet. When you get that updated to a non-beta release could you let me know? I will set some time aside early next week to get my library updated and depending on it.

Thanks!

angularsen commented 8 years ago

Alright, I should be able to merge into master and release a stable nuget this week. A bit hectic with moving to a new house these days, but I don't think this part should take me long.

angularsen commented 8 years ago

@jbienzms #152 now merged, and 3.33.0 stable nuget released. https://www.nuget.org/packages/UnitsNet.WindowsRuntimeComponent/3.33.0

jbienzms commented 8 years ago

Sounds good @anjdreas. I have to be in Austin for HoloLens on Monday and Tuesday, and I have to finish that IoT video up this Friday and next Wednsday. My goal is to have my library live with UnitsNet by end of next week. Sorry for the delay on my end. I'll post back here when it's done.

angularsen commented 8 years ago

No problem, thanks for the update.

angularsen commented 8 years ago

Just following up on this, did you get a chance to test the nuget in your app?

jbienzms commented 8 years ago

I did not get to it last week but I did get to it today. :) I have not pushed an update to the NuGet package out yet but I have pushed an update to source control. You can check it out here:

https://github.com/jbienzms/iot-devices

The reason I haven't pushed a NuGet package yet is because I just published this video about the library on Channel 9 today as part of The Maker Show:

Breaking out of the Loop https://channel9.msdn.com/Shows/themakershow/13

If you get a chance, let me know what you think of the video.

Since I don't have my Raspberry Pi with me I don't have the ability to test my changes and I don't want to push a package update without doing that testing. Especially with that video going live. I should have a chance to do some tests on Wednesday and should hopefully be comfortable pushing a package update then.

I have a related question about a new unit of measure. I'll post that in another thread though to not confuse this one.

angularsen commented 8 years ago

Cool video, I actually learned a couple of new things such as DispatcherTimer and got a bit more familiar with the IoT library, which I haven't explored yet, so thanks!

If you want some constructive criticism, here are a couple of first impressions:

We can continue this conversation elsewhere if you want.

As for testing Units.NET, no rush. I'm just checking in and interested to see how it works out.

angularsen commented 8 years ago

@jbienzms Just curious if you got around to test the nuget in the app? It still seems to be a bug with having to set SpecificVersion to False for UWP apps using packages.config, even with VS2015 Update 3.

jbienzms commented 8 years ago

Yes, I did get around to testing the NuGet. And as you report, it works very well for C# projects but is still having problems in JavaScript apps. Really there's nothing we can do about that. The VS team is aware of the issue and has a bug open for it.

I have incorporated UnitsNet into my project and that is how it is committed to source control. Sadly, I still have not gotten around to releasing an updated NuGet package. It's on the TODO list.

From your end, you can certainly consider this issue closed. I don't think there's anything else you can do on your end to make the process easier. Thank you again for your hard work and for supporting WinMD with your library!

angularsen commented 8 years ago

Very cool, thanks for the update!

jbienzms commented 7 years ago

Just wanted to follow up with you @anjdreas and let you know that I FINALLY published an update to the NuGet package that includes the dependency. As of NuGet package 1.0.8, UnitsNet is referenced by the Microsoft.IoT.Devices library. I'm so very sorry that it took so long to get pushed up to NuGet. Other responsibilities had me pulled in different directions. Having said that, I'm super excited to see the Windows Runtime version has been downloaded over 900 times! Glad to see my library wasn't the only code to benefit from this work.

Thank you again for your support, and keep up the great work on an excellent library!

angularsen commented 7 years ago

Thanks for letting us know. I'm glad to hear this piece of code is useful to you and that you are referencing it in the IoT nuget! Would you mind adding a short snippet for our README section?

https://github.com/anjdreas/UnitsNet#who-are-using-this

jbienzms commented 7 years ago

Sure @anjdreas. Here you go:

Microsoft.IoT.Devices extends Windows IoT Core and makes it easier to support devices like sensors and displays. It provides event-driven access for many digital and analog devices and even provides specialized wrappers for devices like joystick, rotary encoder and graphics display.

http://aka.ms/iotdevices (home page including docs) http://www.nuget.org/packages/Microsoft.IoT.Devices (NuGet package)

angularsen commented 7 years ago

Thanks, can I link to your github username or would you prefer not to? Also, is it OK for me to add the microsoft logo as the other entries have?

jbienzms commented 7 years ago

Feel free to link to: http://aka.ms/iotdevices

As for the Microsoft logo, I'm not sure about that. It was built on MS time but it was a personal project of passion, not something I was "assigned" to do. I wish I had a logo for the project. I'd gladly let you use that.

angularsen commented 7 years ago

That makes sense, thanks. I'll add your text and your links.