Units (humidity, distance)

krwq commented 4 years ago

Since we have experimentally added Temperature I haven't seen anyone complaining about this and I also personally find it very convenient. Since then we have added also Pressure and it also seems convenient.

I think we should make a step forward and add couple more units to avoid (or reduce consequences) breaking changes in the future:

distance
humidity (RH, with perhaps some utils for absolute etc)
(not sure) 3 units for IMU - on one hand Vector3 is fine but on the other hand this could've been more convenient (at the same time this will probably have many open ends)
voltage
current
power

@shaggygi @Frankenslag @ZhangGaoxing @MarkCiliaVincenti @Ellerbach @joperezr @pgrawehr

pgrawehr commented 4 years ago

Since we have also PR #679 which reads NMEA sequences, what about speed, position and angles / direction? Another unit that is commonly used with environmental sensors attached to a Pi is PPM (parts per million) for concentration of gases or air quality.

krwq commented 4 years ago

I'm not sure about derivatives or multidimensional values yet but angles I think is a good idea

Ellerbach commented 4 years ago

distance

Yes, distance would be a great one for sure. To make easy the international to non international units also take correctly in consideration the multiple elements for the international one (cm, m, km, etc).

humidity (RH, with perhaps some utils for absolute etc)

Yes as well, for both relative, absolute ans specific (see https://en.wikipedia.org/wiki/Humidity).

(not sure) 3 units for IMU - on one hand Vector3 is fine but on the other hand this could've been more convenient (at the same time this will probably have many open ends)

I guess issue is more about using the same units than the Vector3 (and 4) which are perfect for this. So units for IMU can be great as well to help having coherence in the units as well as easy way to convert them. That will help for angles (degree vs radians) as well as acceleration in G or milli G.

speed, position and angles

Like for distances, speed is a good one as not everyone is using the international speed system (m/s) and even those using express them in km/h for example for cars. So I'll vote for it.

For position, @pgrawehr can you give me an example? Is it terrestral position with logitude and latitude? Or something else? In case of terrestral one, I would vote for yes as well as there are multiple ways to express them as well

PPM

If this is standard and every one is using it with no convertion, then not sure it's needed.

krwq commented 4 years ago

@Ellerbach the only thing for units as Position/Velocity/Acceleration is that velocity can be anywhere from 1 to 3 dimensional depending on context which I'm not sure how would that be abstracted so being conservative I think we should give it more thought (unless everyone is happy with Position, Position2, Position3, Velocity, Velocity2, Velocity3, Acceleration, Acceleration2, Acceleration3)

Speed is not a vector so can be added (or perhaps should we treat it as 1 dimensional velocity)

shaggygi commented 4 years ago

Referencing....

https://www.ccontrols.com/pdf/BACnet_Units.pdf

https://project-haystack.org/doc/Units

pgrawehr commented 4 years ago

Looking at shaggygi's list: General question is whether we need special types for units that are common, but generally use the same unit everywhere (i.e. Voltage, Current, Power).

@Ellerbach Yes, I was talking about terrestrial position, expressed as latitude, longitude and altitude. This should be a separate object, as it prevents lots of errors with i.e. exchange of latitude and longitude (seen that, been there...). Also, this cannot be expressed as a normal 3d-Vector, since the operations allowed on it are very different.

Ellerbach commented 4 years ago

unless everyone is happy with Position, Position2, Position3, Velocity, Velocity2, Velocity3, Acceleration, Acceleration2, Acceleration3

I understand the point, so maybe yes, not in priority 1 and I'm happy with Position, Position 2, Position 3, etc. Again, agree we should wait then for this one. Also, we may have to implement Kalman filters and other filters for those elements. They are must used in any robotic project. They are used to integrate and treat the accelerometer, compass and magnetometer.

this cannot be expressed as a normal 3d-Vector, since the operations allowed on it are very different

OK, got the point. So then it definitely makes sense to create a specific class with all the needed operations in it.

pgrawehr commented 4 years ago

So then it definitely makes sense to create a specific class with all the needed operations in it.

These operations (i.e. greater circle distance between two points) are complex mathematical operations. There are algorithms (and also implementations) for that, but we'll have to check whether there's one with a compatible license. But that's an implementation detail, not exactly relevant for what we want.

pgrawehr commented 4 years ago

I think we should also have types (or at least an enum) for one-dimensional values, such as Temperature (exists already), Voltage, Power, etc. This will allow a common formatting method, that converts measurements to strings with units, including a readable scaling (i.e. to mA or kV). We could even provide some operations on these units (i.e. voltage times current equals power).

MarkCiliaVincenti commented 4 years ago

I reiterate what I said elsewhere that Microsoft needs a new package just for units.

krwq commented 4 years ago

@JTrotta has suggested to use https://github.com/angularsen/UnitsNet instead of re-inventing the wheel. So far looking at that it seems to me we can re-use the library.

License seem to be compatible. What are your thoughts about it?

@MarkCiliaVincenti do you mean you prefer to re-implement? (assuming you read the thread above looking at timing)

krwq commented 4 years ago

I'm currently asking for a green light if Iot.Device.Bindings can depend on that. Ideally would be nice if we could get that library's namespaced evolve to System.Units but we should start with checking if it works for us first

krwq commented 4 years ago

Got a green light. Plan is following:

[x] let's create a branch in dotnet/iot: feature-units and start integrating that library
- Just created https://github.com/dotnet/iot/tree/feature-units which is in sync with master
as we go we should write down all problems with it if there are any
if we notice this is not working well for us we back off
if we are happy with the branch we go forward with it and merge it into master

krwq commented 4 years ago

Anyone would like to help with integrating this? cc: @pgrawehr @JTrotta @MarkCiliaVincenti

MarkCiliaVincenti commented 4 years ago

I would like to help but I think this falls in the remit of someone who is knowledgeable on physics; I'm purely on software development.

krwq commented 4 years ago

@MarkCiliaVincenti first step is to simply convert what we already have to new library. next step is to integrate more

JTrotta commented 4 years ago

Anyone would like to help with integrating this? cc: @pgrawehr @JTrotta @MarkCiliaVincenti

If someone creates the template/repository, I can help.

krwq commented 4 years ago

@JTrotta already created a branch in this repo (feature-units). First step would be to remove our Units library and add package reference to Iot.Device.Bindings to UnitsNet and replace all failing places with new library and equivalent APIs

JTrotta commented 4 years ago

I see. So you do not want to create a new library, but integratate it into IOT, do you? You want remove Iot.Units namespace and src under Device folder, and start to refer to the external to check functionalities or you want to integrate source code?

Sorry for questions, but I'd like to better understand your aim.

MarkCiliaVincenti commented 4 years ago

I see. So you do not want to create a new library, but integratate it into IOT, do you? You want remove Iot.Units namespace and src under Device folder, and start to refer to the external to check functionalities or you want to integrate source code?

Sorry for questions, but I'd like to better understand your aim.

Yeah, I didn't understand either.

pgrawehr commented 4 years ago

See coment from krwq above ("got green light"...)

He has created a branch for integrating that library. The task for whoever picks this up are first:

Remove our own classes (Temperature and Pressure pressumably)
Add reference to the UnitsNet package
Try to build
If it works, submit a PR against the "feature-units" branch

Then we can try to make more use of that library and find whether it misses features we need or has other issues.

krwq commented 4 years ago

exactly as @pgrawehr has wrote, sorry for confusion 😄

krwq commented 4 years ago

I'll start with initial PR

krwq commented 4 years ago

Let me know what you think. So far I like the result of this experiment and I think we should keep going with integrating the units. @joperezr?

krwq commented 4 years ago

if we decide we like it I think we should go with some multidimensional units (so perhaps convert IMUs) and see how that code looks like. After that if we decide we want this permanently we should merge that into master and work on master directly

krwq commented 4 years ago

Ok, I have just tried converting BNO055 to use MagneticField but couldn't see any way of creating a 3 dimensional vector out of that with UnitsNet. I've created an issue there to see what they recommend: https://github.com/angularsen/UnitsNet/issues/780

krwq commented 4 years ago

anyone interested in continuing the effort? the multi-dimensional units seem to be out of scope for now (.NET currently doesn't allow to put restrictions on generic types such that you could restrict type to numbers only and use operators which seem to be the reason why UnitsNet haven't added any multidimensional units - they have some workaround in progress and I also asked our PM if this is something they plan for the future).

we should focus on finding which units can be integrated into our repo and do some of that integration.

pgrawehr commented 4 years ago

You mean that we should find out where we would rather use units instead of plain doubles? I guess that needs going trough all the bindings and check that. My "big" IOT project is taking some time now as well, but I'll try to find some time for a few known bindings later. And I'm still also working on the Nmea stuff, which will use many units.

How do we avoid introducing all kinds of breaking changes? Duplicating properties is probably not a good idea (i.e. existing "Voltage" property and now "VoltageAsRealVoltage").

krwq commented 4 years ago

Yes for the question. For the breaking changes: I personally feel flexible here but my opinion is that for Iot.Device.Bindings we should go with breaking changes although reducing them to as little as possible (if any property had unit in the name and returns double then it should still be there but if it is called say Voltage which would clash with name we'd like to use now I think we should change the type in those cases). People have a choice of still using the older versions of the package but in order to move forward they will need to do some minor changes... I think we'd do the same if we've added more units to our Units and I think this is something we should do.

joperezr commented 4 years ago

How do we avoid introducing all kinds of breaking changes? Duplicating properties is probably not a good idea (i.e. existing "Voltage" property and now "VoltageAsRealVoltage").

Regarding that it is worth noting that Iot.Device.Bindings package doesn't have the same level of guidelines that System.Device.Gpio does, and we do allow making breaking changes as long as they are valuable and it is easy for consumers to adapt to the new breaking changes which I believe we are fine on both of those items with this.

EDIT: looks like @krwq beat me for a few seconds haha, but answer is pretty much the same. 😄

pgrawehr commented 4 years ago

@krwq: Can you please rebase the branch? The removed platform dependency will remove issues with VS playing havoc on reference assemblies, which is ugly to work with if one needs to go trough a lot of bindings.

joperezr commented 4 years ago

Can you please rebase the branch? The removed platform dependency will remove issues with VS playing havoc on reference assemblies, which is ugly to work with if one needs to go trough a lot of bindings.

Done. Make sure whoever is going to work on this to do a fetch again in case they had already pulled down the branch as when you do a rebase the history is changed so local branches will be broken and will need a re-fetch.

krwq commented 4 years ago

feature-units branch is merged now which means we are officially using UnitsNet now. I'll close this issue and replace it with issue about adopting this library better

dotnet / iot

Units (humidity, distance) #869