-
-
Notifications
You must be signed in to change notification settings - Fork 184
Implementing display units system in metadata #2225
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
base: master
Are you sure you want to change the base?
Conversation
|
Shouldn't some of this be in the specification? Now we have to manage all of these paths here too? |
|
Are the converted values only meant as Data Browser view only, or is to be sent as subscribers as delta? @motamman There are a few good and faste libraries that do unit conversions. They don't tend to change m much so they are not really a dependency issue. |
|
Yes, this should be in the spec, but imho we should do implementation first, then spec. So let's first concentrate on the mechanism here. I think we should create |
|
Server APIs are to stick to unconverted, per spec values. |
The data browser conversion is just a parlor trick. I can see why that would be an attractive idea, but I tried a few when I first started messing with this idea a few months ago, and they all had gaps. (I can't remember exactly what right now.) I basically culled what is included here from a few sources. There's no reason a client couldn't use one of those libraries, but to your point, these conversions are stable, so once they are set, there isn't much to maintain. Am I answering your question? |
|
From SKipper client perspective it looks good!
Thank you! |
The formulas are designed for mathjs, not JS eval(). In the example I baked into the databrowser, I meant to use Math.js.evaluate(formula, { value }) to safely evaluate conversions. (I just noticed I didn't push that.) But mathjs is a sandboxed math expression parser - it can't execute arbitrary code, only mathematical operations on the value variable we pass in. So "value * 1.94384" works, but "process.exit()" would just throw a syntax error. We could prophylactically add server-side validation, but the math.js already addresses the core security concern. Additionally, I didn't want to include it in this, but I will be publishing companion signalk-advanced-units-preference when this becomes a thing. It will handle myriad date/time conversions, allow custom units, conversion formulas, and categories, and support other custom stuff. I do not anticipate any of that customization being baked into SK. But given that, maybe we should add server-side validation. Again, did I understand your concerns? |
Yes for units conversion librairies. With Mathjs in the picture it makes more sense a library might be harder to find. Thanks |
Yes, thank you! :-) |
|
I think we should add server-side parsing of the expressions with mathjs and reject ones that don't compile. This should include injected metadata. DataBrowser should use https://mathjs.org/docs/expressions/parsing.html#compile and a singleton cache of the expressions. Another opportunity would be to provide a small library that has true js functions for all the well-known conversions and fallback to mathjs for dynamic ones. Needs rebase & changes, DataBrowser changed underneath. |
Your first two points are spot on and not difficult to do. I think the third point would add negligible optimization, given that we'd be compiling everything, but it would also sacrifice a lot of maintainability. If there is consensus, I will try to finish that up this week. |
|
One note here, maybe you want to have one more preset called "nautical" mostly for Europe guys (maybe other regions). We in Europe might use knots for wind speed, SOG, nautical miles for distance, but use meters for depth :-) SKipper does have this as preset, it was requested by sailors here in Europe. |
I agree, and as an immigrant sailor in the US, my preferred units bounce across various hierarchies. I can put together a nautical, non-us preset, |
I think you can do that without any hit to maintainability - just implement a conversion library that allows you to get a conversion function by the formula string. You don't care if it uses mathjs or something else. But this is a sidetrack, not needed to proceed with this. |
I forgot about this. On it. |
How would we do this? Publish both base units values and meta , plus converted values and converted meta? |
Just base units and meta, which would include base units and preferred units, and conversion formulas from base and to base (should you want to PUT data back) SK would not do any conversions. For your use case, all of the user-selected conversions in KIP would go away, and you would use import { convertValue } from '@signalk/client2' to load the JS that would do the conversions. It would require some refactoring, but then it would completely eliminate the need to manage unit conversions. At least that is what I THINK @tkurki is suggesting. If you like, I could throw together an example of how to refactor a current KIP widget to illustrate the refactoring. The big benefit comes for future development, which will not have to worry about managing conversions, and for users, who will have to select their presets once and, if fully adopted, will see those settings reflected in all SK-related apps. |
Exactly. Or go even go one step deeper and support subscribing to data from a context-path-$source combination, some thing like |
|
If we go with central unit consersion, I think the design has to account for the many cases where we need to use different units for the same path. Like different units for KIP in different widgets, or maybe different users with different preferences connected to the same server, units used by hardware (sensors and such) that can't be changed, sk to sk transfert, just to name a few. Maybe converting all path values to multiple units in realtime can be costly for the server. Not sure... Also to keep in mind is, it's honestly trivial to implement units conversions with libraries and their formulas have been vetted by many. But I like the idea of seing converted values in the Data Browser. |
|
@motamman I made changes to my previous comment. Cross fire...sorry. I see your point. I had wrongly understood that it would do conversions on the server. It would be nice to set preferred units on the server. Isn't the preferred units a users setting more than a server setting then? |
There is nothing to stop you from keeping your code exactly as it is. The data would be exactly as it was. This is not a central conversion system. This just adds default conversions in the metadata. It does not do the conversion. The CPU cycles are on the client, not on the server, just as it is now. You can leave KIP exactly as is. Or you could use the default display preference to select a default selection in the widget setup and allow the user to select something else. The widget might not need all those clicks to set up, unless there were desired changes. In my mind, the issue is less about the developer and more about the user. They set what presets they want and can forget about it. That is just a better experience than having to change the settings for every app. Set it once for Freeboad, another time for KIP, another time for SKkipper, another time for WillhemSK and so on and so on into the future. And what about incoming data? This contemplates the inverse, saving the data from some other unit to a base unit. KIP has all the scaffolding in place to manage units, but that is just another thing to maintain, and future apps will not have to bake it in. You are right that this is trivial, in some ways, but I think SIgnalK system should have a mechanism for setting default display units. |
Gotcha. That threw me for a bit of a loop. I thought I was losing my mind. No worries. Pax |
|
@motamman FYI it's truly not about KIP. I'll do what needs to be done :) What about the idea of attaching the preferred units to the user and have the Users page handle this as opposed to a "server settings" page or section? Then Data Browser checks what the current logged in user preferred units are and converts for display. For consumers (Apps & plugins), when we login, the server could send back the user's preferred units un the response, and/or have an v2API to get that info. KIP has preferred units per profiles (since its bound to users it's a bit similar to user preferences) with the ability to override at the widget level. I would rather pull the user preferred units from the SK user properties at login. |
I like that idea, but what about devices? What presets would they inherit? (Not an entirely well-formed thought, but i have used registered devices almost as a proxy for users.) |
|
@motamman devices could user the signed in user's preferred units, if they use usr/pwd. Some use device tokens so those would not work. But my guess is most devices send data and in that case they should use SI units. |
I have actually used device registration for my Android app like sudo users. One client device at the helm. another nav, another in the cabins etc. They have separate setups, but anybody can use the devices. Not a problem for today. I just took a quick breeze through, and I don't think what you want is difficult at all, but I would rather get the core functionality going and then onto some other stuff @tkurki has suggested. |
|
It was just a suggestion that I think would raise the adoption for this PR. Up to you. |
…tegory for SignalK paths
…DataBrowser and unit preferences API
|
Just a quick note that I talked with motamman (about this and other things sk), he'll continue working on this:
|
|
@cmotelet have you looked into this work? any insights to share from unit prefs in InstrumentPanel? |
|
Oh sorry, I missed this conversation, so I'm a little late to the party ! As a rule, I've always preferred server-side work than to doing the same thing multiple times on multiple clients. At least this way, once the client has made their choice of preferred units, regardless of the client, they will have the same display. To take this further, I would like to be able to consume a path directly in the unit desired by the customer rather than converting it once it arrives on the client using the formula provided in the metadata. A lot of work has been done in the data browser, but I'm not sure that's where "John the boater" will go to find their metrics translated into their preferred unit. |
Sorry, @tkurki, I didn't mean to create this PR, but since I did, this is what I have been noodling over. This is entirely inside the SK framework. My advanced features would be separate in a new plugin/wedapp. (I know you hate my non-descriptive branch names, so I changed pillowtalk to something more functional)
I tried to document it all in a README in the PR.
And since I was in there, I took the liberty of dressing up the metadata editing page. (I took styling cues from the recent redesign of the plugin config page)
And I added both base and display units side-by-side in the standard data browser listing.
Here is an example of http://localhost:3000/signalk/v1/api/vessels/self/navigation/speedOverGround endpoint with metadata:
{
"meta": {
"units": "m/s",
"description": "Vessel speed over ground. If converting from AIS 'HIGH' value, set to 102.2 (Ais max value) and add warning in notifications",
"displayUnits": {
"category": "speed",
"targetUnit": "kn",
"formula": "value * 1.94384",
"inverseFormula": "value * 0.514444",
"symbol": "kn"
}
},
"value": 0,
"$source": "signalk-mqtt-import.GN",
"timestamp": "2026-01-04T16:38:47.000Z",
"sentence": "RMC",
"values": {
"signalk-mqtt-import.GP": {
"value": 0,
"sentence": "VTG",
"timestamp": "2026-01-04T16:38:47.779Z"
},
"signalk-mqtt-import.GN": {
"value": 0,
"timestamp": "2026-01-04T16:38:47.000Z",
"sentence": "RMC"
}
}
}