Skip to content

Support nanoFramework #837

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

Merged
merged 24 commits into from
May 7, 2021
Merged

Conversation

Ellerbach
Copy link
Contributor

@Ellerbach Ellerbach commented Sep 17, 2020

Fixes #836

Adding single unit creation to support usage in nanoFramework.

This is an advance draft, things to discuss, see below on what needs to be done. Good news if that it works quite smoothly all up :-)

What is done:

  • Logic to generate single files for Units and Quantity
  • Creation of specific project per unit (nfproj)
  • Shared components (package.config and AssemblyInfo.cs)
  • Creation of a master sln project
  • Managed specificities of nanoFramework to make things really much lighter, replaced Math.PI and some Math.Pow byt the real value (to avoir issues with nanoFramework Math.Pow with double as well as accelerating the conversions)

What needs to be done:

  • There is no operator support for decimal type so far in nanoFramework, 4 units impacted. The files are generated, the projects as well, we can decide to get them out the time to solve the issue on the nanoFramework side if needed
  • A specific build setup for those single units, so far, the build fail as the path are not properly supported from the main Directory.Build.props. Comment out:
<!-- <OutputPath>$(MSBuildThisFileDirectory)Artifacts/$(MSBuildProjectName)</OutputPath> -->

This will allow all projects except 4 to compile (see the issue with Decimal operations)

  • Install automatically the nanoFrmaweork nuget package

@codecov-commenter
Copy link

codecov-commenter commented Sep 17, 2020

Codecov Report

Merging #837 (7608a53) into master (3bca803) will increase coverage by 82.8%.
The diff coverage is n/a.

❗ Current head 7608a53 differs from pull request most recent head c8acf13. Consider uploading reports for the commit c8acf13 to get more accurate results
Impacted file tree graph

@@            Coverage Diff            @@
##           master    #837      +/-   ##
=========================================
+ Coverage        0   82.8%   +82.8%     
=========================================
  Files           0     291     +291     
  Lines           0   44158   +44158     
=========================================
+ Hits            0   36538   +36538     
- Misses          0    7620    +7620     
Impacted Files Coverage Δ
...ratedCode/NumberToElectricPotentialExtensions.g.cs 100.0% <0.0%> (ø)
...tsNet/GeneratedCode/Quantities/ReactiveEnergy.g.cs 77.3% <0.0%> (ø)
...sNet/InternalHelpers/ReflectionBridgeExtensions.cs 46.2% <0.0%> (ø)
UnitsNet/GeneratedCode/Quantities/Capacitance.g.cs 79.9% <0.0%> (ø)
UnitsNet/CustomCode/UnitParser.cs 86.6% <0.0%> (ø)
...et/GeneratedCode/Quantities/LuminousIntensity.g.cs 75.0% <0.0%> (ø)
...Code/NumberToRotationalAccelerationExtensions.g.cs 100.0% <0.0%> (ø)
...Serialization.JsonNet/UnitsNetBaseJsonConverter.cs 100.0% <0.0%> (ø)
...eneratedCode/NumberToSpecificVolumeExtensions.g.cs 100.0% <0.0%> (ø)
...sNet/CustomCode/Quantities/SpecificVolume.extra.cs 100.0% <0.0%> (ø)
... and 281 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3bca803...c8acf13. Read the comment docs.

@Ellerbach
Copy link
Contributor Author

So easiest way is just to remove the 4 units that are decimal based. They won't be anyway used by any sensor.

@angularsen
Copy link
Owner

angularsen commented Sep 17, 2020 via email

@Ellerbach
Copy link
Contributor Author

decimal to double for these quantities and maybe somehow skip the largest/smallest units that caused problems with round-trip calculation tests

I was thinking of that but then all the formulas will have to be adjusted. like " * 8m" should be transformed, and all the others as well. Removing large units.
Regarding the units, I'm not sure about the value vs time spent. It would be a Length or a Mass or a Speed, then yes, worth spending some time. Now it's Information, Power, BitRate, so, easiest is to leave them like they are. Especially as they are not real life measures, so chances a sensor or anything would use them directly is low. That's different from the Ratio one used for humidity sensors for example.

@angularsen
Copy link
Owner

Fair enough, can always attack those quantities later.


By default, the code generator will as well generate automatically 1 project per Quantity.

You can load all the projects at once using the automatically generated solution ```nanoFramework.UnitsNet.sln```
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UnitsNet.nanoFramework.sln

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could also add a bit more information about what nanoframework is and what it is used for (just a few sentences) and link to the official nanoframework site from here.

@@ -43,7 +43,8 @@ internal class Program
/// <param name="verbose">Verbose output? Defaults to false.</param>
/// <param name="repositoryRoot">The repository root directory, defaults to searching parent directories for UnitsNet.sln.</param>
/// <param name="skipWrc">Skip generate UnitsNet.WindowsRuntimeComponent? Defaults to false.</param>
private static int Main(bool verbose = false, DirectoryInfo repositoryRoot = null, bool skipWrc = false)
/// <param name="skipNanoFramework">Skip generate nanoFrmaework Units</param>
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo.
Maybe: Skip generate UnitsNet.nanoFramework quantities? Defaults to false.

@@ -31,3 +31,9 @@ CodeGen.exe --ver

Hit TAB and it should now suggest `--version` and `--verbose` parameters.
This should work with any .exe that is compiled with Dragonfruit's app model.

## nanoFramework project generation
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe just call this ## nanoFramework.
We should also mention this supported framework in the Build Targets section at the top of the readme, with a link to this section with more information.

/// <param name="quantities">The quantities to create</param>
public static void Generate(string rootDir, Quantity[] quantities)
{
var outputDir = Path.Combine(rootDir, "NanoFramework", "GeneratedCode");
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would change the directory name to UnitsNet.NanoFramework to be consistent with WRC and other projects.

Log.Information(sb.ToString());
}

GenerateSolution(quantities, Path.Combine(outputDir, "nanoFrmawork.UnitsNet.sln"));
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change to UnitsNet.nanoFramework.sln

private static void GenerateProject(StringBuilder sb, Quantity quantity, string filePath)
{
// This will have to be adjusted
// $(MSBuildToolsPath)..\..\..\nanoFramework\v1.0\
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this comment still relevant? Seems already adjusted.

@angularsen
Copy link
Owner

@Ellerbach I updated the build scripts, but msbuild fails with error MSB4057: The target "Build" does not exist in the project.

Probably need some dependencies to build this from AppVeyor and commandline, but I couldn't immediately see from the nanoframework docs what dependencies are needed?
They list extensions for VS and VSCode, but we preferably need something for msbuild or dotnet build to work from command line without VS/VSCode. Do you know how to make that work?

https://docs.nanoframework.net/content/building/build-in-visual-studio.html
https://docs.nanoframework.net/content/getting-started-guides/getting-started-managed.html

@Ellerbach
Copy link
Contributor Author

Probably need some dependencies to build this from AppVeyor and commandline, but I couldn't immediately see from the nanoframework docs what dependencies are needed?

Any of the nanoFramework nuget uses an automated chain: https://github.com/nanoframework

@josesimoes can for sure help here to find the right things to do.

@josesimoes
Copy link
Contributor

josesimoes commented Oct 4, 2020

Hi @angularsen!

In order to build a nanoFramework project the VS extension has to be installed, otherwise you'll see that error above.
Because installing a VS extension can be problematic on CD-CI environments we have a workaround for it, which is basically cheating and installing just the required components for msbuild.

As we are using Azure Pipelines, we have an Azure Pipeline task that does exactly that. You are using AppVeyor which, unfortunately, doesn't allow you to use it directly. This is basically a PowerShell so you shouldn't have much problem adding it to your build. Please check the code here

Also know that before Azure Pipelines we were using AppVeyor too. Check the yaml that was used back then:
https://github.com/nanoframework/lib-CoreLibrary/blob/a160f28c367cae5c6d38fdb52ec84124204b7622/appveyor.yml
(at that same point in history you can grab the ps file that was used)

Hope this can be helpful. If you get stuck on anything just ping me and I'll be glad to help you.

@angularsen
Copy link
Owner

Thanks for the pointers. I'm afraid my work is taking up all my time these days. I won't be able to push this forward anytime soon. If you are interested in trying to set up a working appveyor pipeline and do a pull request, I can assist on that.

@Ellerbach
Copy link
Contributor Author

Thanks for the pointers. I'm afraid my work is taking up all my time these days

I have the same issue regarding time. So let's see when we'll both have a bit more time.

@stale
Copy link

stale bot commented Jan 14, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Jan 14, 2021
@stale stale bot closed this Jan 23, 2021
@josesimoes
Copy link
Contributor

@angularsen I can look into the required changes in your pipeline to add a .NET nanoFramework build.
Should I go ahead? Or you prefer that we just fork the repo and handle that at our end with a pipeline of our own?

@angularsen
Copy link
Owner

angularsen commented Apr 9, 2021 via email

@josesimoes
Copy link
Contributor

josesimoes commented Apr 12, 2021

@angularsen this turned out to be even easier than what I tough initially. All because the init and build are using Power Shell. I was under the impression that this had to be deal with Appveyor yaml... 😉

The PS1 have been updated along with the Solution to build the nanoFramework projects.
If you can please open the PR again to have the build running and see how it goes on your build system.

@angularsen angularsen reopened this Apr 12, 2021
@stale stale bot removed the wontfix label Apr 12, 2021
@josesimoes josesimoes force-pushed the nanoFramework branch 3 times, most recently from 5645bbd to 7608a53 Compare April 12, 2021 14:17
@josesimoes
Copy link
Contributor

@angularsen build 🟢 ! Everything working except the NuGet packages generation. 👏🏻

@angularsen
Copy link
Owner

Holy cow, what an effort @josesimoes !
I've skimmed the commits and think I'm following what is going on. It is some pretty crazy stuff.

I don't currently have Visual Studio installed so I can't test locally right now.
I see AppVeyor is failing on this

.\\build-with-wrc.bat : Error NU5008: Manifest file not found at 'C:\projects\unitsnet\UnitsNet.NanoFramework\UnitsNet.NanoFramework.nuspec'

Do you need my help on anything on the NuGet end of things, or will you work on that next?

Great job!

@josesimoes
Copy link
Contributor

Holy cow, what an effort @josesimoes !
I've skimmed the commits and think I'm following what is going on. It is some pretty crazy stuff.

Awesome! If something is not clear, just ping me or @Ellerbach .

I see AppVeyor is failing on this

.\\build-with-wrc.bat : Error NU5008: Manifest file not found at 'C:\projects\unitsnet\UnitsNet.NanoFramework\UnitsNet.NanoFramework.nuspec'

Do you need my help on anything on the NuGet end of things, or will you work on that next?

Just waiting on a decision about the nuget strategy. Please refer to the latest comments on #836.

Great job!

Thanks! 😊

@tmilnthorp
Copy link
Collaborator

Does nanoframework support trimming of self-contained applications like netcore by any chance?

@josesimoes
Copy link
Contributor

Does nanoframework support trimming of self-contained applications like netcore by any chance?

It has a similar feature, yes. It's a post-build processing that removes the code that's not called in an app.
I understand what you're trying to accomplish but in this case it won't work because it's a library.
When the PE file of a library it's parsed nothing is removed because... it's a library... everything has to be made available to an app referencing it.

@josesimoes
Copy link
Contributor

@angularsen any decisions on the nuget strategy ? Are we good with adding nuspecs for each unit?

@tmilnthorp
Copy link
Collaborator

Does nanoframework support trimming of self-contained applications like netcore by any chance?

It has a similar feature, yes. It's a post-build processing that removes the code that's not called in an app.
I understand what you're trying to accomplish but in this case it won't work because it's a library.
When the PE file of a library it's parsed nothing is removed because... it's a library... everything has to be made available to an app referencing it.

I was thinking more for the final application. Even if the nuget package is large, are the final targets automatically trimmed?

angularsen and others added 20 commits May 7, 2021 09:37
error MSB4057: The target "Build" does not exist in the project.

Need some dependencies to build this.
- Move nfproj files.
- Fix files path in all projects.
- Fix references in all projects.
- Fix project path in solution file.
- Fix typo in solution name.
- Fix nfproj content to new location.
- Project GUID is now generated on project.
- Solution builder now places nfprojs in their own directory and grabs project GUID from project file.
- Now projects requiring Math NuGet packages are properly processed.
- Remove PackageGenerator as it's not required anymore.
- Should be along with the other sln files of the project,
@josesimoes
Copy link
Contributor

@angularsen just added nuspecs for the other Units identified by @Ellerbach as the ones used in .NET IoT.
Suggest that we go "live" with these ones, gather feedback and/or comments and then move to add the rest.

Because this has been branched in a while, I've rebased it so it's now synced with the master branch.

Considering that in 2 weeks time there will be a hackathon where these are required as part of the work happening to have .NET IoT bindings in sync with .NET nanoFramework, it would be great if these could be published in the coming days.
Let me know if anything else it's required to make this happen.

@angularsen
Copy link
Owner

✅ Build and tests on AppVeyor is green
Nuget is packed and looks OK, but I can't really test it.
✅ Skimmed through the diffed files, looks good.

GitHub can't even load the list of changed files 😆

image

I spotted a few minor things, but nothing blocking a merge.

Looks good to me, let's give it a try!

Temperature nuget should be published by AppVeyor within 20 minutes or so.

@angularsen angularsen changed the title Adding single unit creation to support nanoFramework Support nanoFramework May 7, 2021
@angularsen angularsen merged commit 2f6dc63 into angularsen:master May 7, 2021
@angularsen
Copy link
Owner

angularsen commented May 7, 2021

@angularsen
Copy link
Owner

angularsen commented May 7, 2021

A couple of observations so far:

Nuspec version is not increased with UnitsNet

Build\bump-version.bat increase the version number of UnitsNet and UnitsNet.NumberExtensions.

The latest build 4.90.0 still built 4.89.0 of nanoframework nuget, so I guess we need to increase all the .nuspec files similar to how we do for WindowsRuntimeComponent in Build\set-version-UnitsNet.ps1.

AppVeyor warning

Consider renaming to Invoke-Foo or some other fitting verb.

WARNING: The names of some imported commands from the module 'build-pack-nano-nugets' include unapproved verbs that 
[00:00:31] might make them less discoverable. To find the commands with unapproved verbs, run the Import-Module command again with
[00:00:31]  the Verbose parameter. For a list of approved verbs, type Get-Verb.
[00:00:31] Initializing...

Inline console output when running build script

When running build.bat.

Instead of:

[00:00:53] [10:57:51 INF] TemperatureChangeRate:             unit(OK) quantity(OK) project(OK) 
[00:00:53] [10:57:51 INF] Creating .NET nanoFramework project for TemperatureDelta
[00:00:53] [10:57:51 INF] Package(OK)

Maybe something like this.

[00:00:53] [10:57:51 INF] TemperatureChangeRate:             unit(OK) quantity(OK) project(OK) nanoFramework(OK) Package(OK)

Or we can just omit the entire OK-thing, I don't consider it very useful anymore. It was mostly helpful in the early days while debugging and developing the code generator.

Minor whitespace issues in nuspec

Double whitespace in Authors: Andreas Gullberg Larsen, nanoFramework project contributors

Remove whitespace/newline in Description:

Adds temperature units for Units.NET on .NET nanoFramework.
 For .NET or .NET Core, use UnitsNet instead.

Artifacts\UnitsNet.zip has all nano projects in root

Expected to find these under UnitsNet.NanoFramework folder maybe.

image

https://ci.appveyor.com/project/angularsen/unitsnet/builds/39061929/artifacts

@josesimoes
Copy link
Contributor

A lot of stuff to improve there... I'll get into it ASAP. Most of that are "just" copy/paste from what was there...

On the nuget version, for example, that could (should?) be passed in the command line instead of being fixed in the nuspec file.
We do that in nanoFramework build and that works great.

Also, on versioning, we have it automated by using the nice nbgv.
Even msbuild uses it 😉

@josesimoes
Copy link
Contributor

Nuspec version is not increased with UnitsNet

Build\bump-version.bat increase the version number of UnitsNet and UnitsNet.NumberExtensions.

The latest build 4.90.0 still built 4.89.0 of nanoframework nuget, so I guess we need to increase all the .nuspec files similar to how we do for WindowsRuntimeComponent in Build\set-version-UnitsNet.ps1.

Actually I haven't done anything to bump the nuspec version, so no surprise that the version doesn't get updated 😅 . Fix is on the way.

AppVeyor warning

Consider renaming to Invoke-Foo or some other fitting verb.

Well... that's not a very fair warning... according to that documentation Build is an approved verb, you can see it here. I guess the issue is that the PS version being used in AppVeyor is not 7.1 therefore the warning.

Inline console output when running build script

When running build.bat.
I'll go with the simplest approach and drop the OK + verbose output.

Minor whitespace issues in nuspec

Double whitespace in Authors: Andreas Gullberg Larsen, nanoFramework project contributors

🔍 can't see any double whitespace there...

Remove whitespace/newline in Description:
OK.

Artifacts\UnitsNet.zip has all nano projects in root

Expected to find these under UnitsNet.NanoFramework folder maybe.
OK.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

nanoFramework Nuget automatic generation
5 participants