-
Notifications
You must be signed in to change notification settings - Fork 69
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
feature: add displayUnits to meta #468
Conversation
@tkurki @sbender9 could you have a look at the final comment in the discussion on #464 (comment) ? Then we can do something with this PR... |
@bkp7 I'd say we merge this now and get some code up on the server to be able to set these kind of metas for an entire server, otherwise we have this meta and it's not used ever (in which case I'd say it makes more sense to just get rid of it). See my last comment on the issue. Thoughts? |
Not surprisingly I'd say merge it now. If it's not in the spec no-one can use it. For me this is the final bit that really leverages the whole meta idea and I think it's the final item in the meta project. It would of course be good to get it in the node-server even if it's a simple implementation as I suspect some develop against that rather than against the spec. It would also put it on all the pi's... |
For the record: I do not think it is a good idea to litter the metadata with arbitrary units and conversions and repeat the same conversions all over the full model. There is room for misuse here as a calibration/adjustment tool rather than conversion. If this is deemed a good idea the conversions should be stored centrally and referenced from meta. Furthermore I would rather solve the problem of display unit preferences for different semantic quantities: depth vs short distance vs long distance. As for software support you can already add arbitrary entries under meta with defaults mechanism in the refrence server. I totally disagree with adding stuff first to the spec so that people can use them. We should build some functionality, server or client or both, figure out the details on how things should work and once verified add them to the spec. The current spec has enough stuff that seemed like a good idea but don’t work all that well in practise. |
Good point, so what do you propose @tkurki? If we follow that line, I'd say we should close this PR until someone (@bkp7?) has the time to prototype this in server, figure out if the proposed spec matches the use cases. |
Some insight into how I arrived at this approach: Q: Is it a good idea to have a way to set vessel wide display unit preferences? I believe there is agreement here that it is. Q: Where should the preferences be distributed from? It could be a peer to peer protocol between consumers, this would currently have to be proprietary. Should it be on the server? I think given the objectives of SignalK it does sit within the SignalK specification and being vessel wide it logically sits as a server setting. Q: Should it be in metadata? As explained in my post #464 (comment), displayUnits along with displayName, longName and shortName are distinctly different to all other meta data in that they are user (vessel) language/locale preferences. Therefore they logically should/could be elsewhere, but 3 of them already exist within meta. We could separate them out but that would need delta support and so on. Q: Should the specification define a list of conversions/displayUnits? This seemed like a good idea, except that the list gets very long and possibly will never be complete particularly when you look at foreign markets. We are up to 140 in our own server code so far and we have only covered US and European units. Should we require all consumers to implement all these conversions even if the product is aimed at only one market (eg. a product marketed in the US should have chinese conversions?). If not we need to make sure missing conversions are dealt with nicely (not difficult but is a potential source of errors). The proposal for units to have a concept of quantities results in arbitrary distinctions. Taking the depth/short distance/long distance example, these choices are based on an underlying individual/regional preference. Short/long means different things to different people µm, mm, cm, m, km? Depth is picked out as one exception to long/short distance. What if a user wants depth in fathoms, draft in feet, distance to WP in Nm, propeller pitch in inches? Things which could be added in future: light height in m, particle size in µm. Similar problems exist with other metrics. For me using such a system is fine at implementation or user interface level but not practical baked into the protocol. We considered a system of specifying particular conversions in a centralised way ie:
and referenced:
This looks neat and the use of something like 'shortLength' is arbitrary and provides complete flexibility. However there are some issues to consider: we need a delta format to convey changes to the displayUnitConversions table and the timing of providing this to consumers needs to be coordinated with the provision of meta deltas (think of shortLength being changed). Once you are into timing issues between different manufacturers' code it is prone to bugs... So we came to this proposal on the basis that if displayUnits are specified, their very presence in the meta gives the consumer everything it needs to do the conversion. No requirement to maintain a lookup, no requirement that the conversion on each consumer implementation is identical, etc. This results in: "meta": {
"displayUnits": {
"display": "RPM",
"description": "Revolutions per Minute",
"factor": 60
}
} Yes it's repetitive, yes it's slightly more verbose than just a reference, but given the number of times meta is transported that's not really an issue. The repetitive stuff is not something that will ever be seen, or have an impact on users. We felt the benefit of simplicity and reliability between servers and consumers tipped the balance. At the end of the day what we are trying to achieve is the setting of vessel wide defaults. If the consensus is that the centralised approach is the way to go, that's fine we will prepare an alternative PR, I have to admit that opinion within our team is split.... Finally I feel there may be a conflict here between work on the specification standard and work on node-server which is an open source implementation of the standard. Is it the case that if we sponsor a change to the specification, it will only be accepted if we code a competitor's (ie. node-server) implementation of that standard? |
Have you considered how the user would configure the vessel wide conversions, given a blank slate? Does the user enter the conversions manually, or are they expected to magically appear? If the user adds a new conversion for one path does the user need to enter it manually for all the paths where that conversion is to be used - or is the client software expected to download the full data and mine it for meta properties with convesions and then produce a "known conversions" list to help the user? What if the user typoed the conversion factor so that it is different in multiple places? Are all conversions linear so that they can be expressed with In the short run I'd be much more happy with an API structure that would allow a client to
Your proposal addresses only handling per individual path. To drive a reasonable UI for that you need a global list of conversions: see https://github.com/SignalK/instrumentpanel/blob/master/lib/ui/settings/conversions.js#L20 for a practical example. I don't get the In the longer run I'd be even more happy with something that would allow me to pick conversions/target units based on semantic classification: see #130 and there the linked strawman implementation - not per individual path. |
I feel that it is very important that we have some implementations of a feature, end to end, in this case spanning the UI as well, so that specification additions and amendments are not just thought experiments. You may have put a lot of effort into this (as clearly witnessed in your contributions so faw), but your inhouse processes and thoughts are not visible to others. For this case it would pretty easy to devise simple web apps, possibly adjust existing ones, to demonstrate how the vessel wide preferences should work. A set of integration tests, possibly in the form of use case scenarios, that exercise and demonstrate the way the functionality works, would be another way to move forward with more confidence. I did not realise you consider the open source server your competitor. I'd be happy to work on the details of this together, but if I want to be realistic that won't happen before September. It would be great if other @SignalK/core members voiced their views here as well. |
The exact implementation can be left to the server and consumer designers. I would not expect the user to ever enter conversion factors at all. One approach at server level might be that the user selected language/locale gives a preset set of displayUnits. Another could be based on shortDistance/longDistance or whatever the designer wishes. In the case of a sensor/gateway based server the user would need to be given the opportunity to set zones/alerts etc. against each N2K/0183/sensor source anyway, so they could also then chose to pick individual displayUnits for that individual branch if the server design allows and they want to. For an OEM server eg. on an individual piece of kit (eg. battery monitor) or on a whole vessel, the vessel default units could be hard coded. ie a boat built for the French market has vessel wide default language of French and French units. In the consumer the designer can implement unit conversion any way they like just taking the vessel wide setting from meta as a default if it is present. ie. for a user who does not wish to use vessel defaults (eg. a hand held device) the consumer shows everything in different units. There is no need for the list of conversions available on the client to be centralised. If we try to define a complete list of all possible conversions it will become very large and have to be updated in the specification as new ones are required. This way the conversions are not defined anywhere (which is the current situation). Consumers which are already in use have conversions written into their code already and this feature means they also have the possibility to display in vessel default units even if those units are not defined in the consumer codebase. The client software only needs to read the meta relating to the particular branch it wants to display. There is no reason that the list of displayUnits should be the same on the server as on the display device. Indeed, I see the power as being that if a server unit marketed and sold into the French market doesn't implement Fahrenheit, it doesn't prevent an American stepping on that vessel with their own consumer software and seeing temperatures in Fahrenheit if that is their personal preference. There are conversions which are non linear which is why the additional properties are present, eg. power to dB. My proposal envisages 'globally' (at least on the master main server) set conversions albeit implemented by the server designer, rather than dictated by the data transfer protocol. It is important to note that no user would ever see the displayUnits detail. It is just how the server communicates the vessel defaults for a particular branch to the client. The iKommunicate is there because you could have a situation where say shortLength is defined on 1 server in one way (eg. m) but on another server in a different way (eg. inches). Since the 'shortLength' designation is arbitrary there needs to be a way to disambiguate. An alternative would be to use a GUID. This proposal absolutely supports semantic classifications if that's what a server designer wishes to do. Also by area, language, individual branches, anything else or no vessel wide defaults at all. The Specification only covers a standard way for a server to communicate to the consumer how to do conversion to the vessel wide displayUnit. For the Consumer designer it adds 'vessel default (unit)' as an additional conversion to those programmed into the consumer, and the method for doing the conversion will always be known. |
Do we? Why not retrieve it with http when needed? Well, there is the edge case where the user adds a new conversion on one terminal and it should be available for use in another terminal. |
How does the consumer know when it is needed? If a conversion definition changes really we want that to be pushed out to consumers so they display the new setting. The same argument as for the meta deltas proposal. |
This sounds like a misunderstanding. I did not mean that we include all possible conversions in the specification. In fact no conversions should be in the specification. I meant that we define a path where a client can retrieve the known conversions, per source unit: a structure not unlike the one in InstrumentPanel, or like what your proposal adds (sorry about overlooking the dB etc stuff btw). Then implementations are free to provide however complete conversion lists that are appropriate, the conversions can be shared via the server with no duplication. So once something like this:
and use it as reference then per meta:
|
Maybe my example above clears this up? When it is referenced in |
Yes, so your example is not that different to mine. We would need to make sure there was a unique name for each conversion though. We can assume the SI unit is unique (we do have a list of those), but the target unit cannot be relied on to be unique (eg. nm could mean nanometres or Nautical Miles). And we need to deal with having conversions being defined in multiple servers. The other issue is that whilst it's fine to do the lookup when a unit is first encountered there is no way of knowing when/if the conversion definition is changed on the server unless we define a delta for the conversions which pushes out the changes. All of the above is possible to define and would provide the feature; as I said, happy to go with it if that's the consensus. The downside of trying to make this list 'global' is that I prefer consumers be able to do conversions even when the server doesn't support that conversion. In other words I would want to keep the current situation where a consumer has inbuilt conversions it can implement. Also I'd prefer it not to be mandatory that a server implement displayUnits. Just a final point on keeping it simple. My proposal whereby all the information for the conversion is held in one place in the meta without needing lookup tables etc. does help when it comes to reliability and debugging. ie. it is very easy to see from logs what was sent and where problems may be between different manufacturers kit. Edge cases are important... |
Reading through this thread again, I have a feeling that we’re makinng it much more difficult to reach consensus because the proposal is quite broad. In my mind, there are a couple of issues to decide on:
I may have missed one or two details, but I feel that - from a specification perspective - these are the most relevant questions. My thoughts:
Regarding delta’s: I do believe that this should be communicated in real time, but it’s a separate issue. If we choose a global definition with references in meta, the global definition is a Finally, unit ambiguity was mentioned as a problem when dealing with custom displayUnits. The example was NM which could be nanometer or newton meter or nautical mile. I believe that is only a problem with using abbreviations as the well known names (which is a pain anyway). If we stick with their proper names it’s not an issue. |
The problem with not including the actual conversion parameters in the data is that there is no universal way to refer to arbitrary display units - if the meta refers to Ambiguity is not a problem if the server is the master for the conversions and the conversions are stored in one place. |
Do we really need to support real time updates to the conversions - like how often does @bkp7 could elaborate on supporting conversions from multiple servers? Values data between servers must always be in SI units, I see this functionality local to one server. |
@fabdrol your 3 questions are spot on IMO. Q2 & 3 being debated...
My proposal gives: The list is populated with the vessel default display units for that leaf + local units provided by the client. @tkurki regarding real time updates, I am supportive of it along with the meta deltas because it means that during setup, changes are immediately apparent on consumer devices. This gives user confidence and a much better experience. The thought of having to do a power cycle or a refresh function on each consumer "to see what it looks like" doesn't appeal. The ambiguity surrounding naming conversions, assuming we go for global conversions, needs to be solved; either by having a prescribed list in the specification or making sure each is globally unique (hence my iKommunicate in the example above), use GUIDs, or agree a single authoritative source. As @turki says if there is just one master main (as per spec) and that becomes THE central and only server listing display units (and serving data) that is fine. However there are 2 problems we see with this:
So if we decide that displayUnits (along with things like MMSI, and Vessel Name) MUST be accessed only from the Master Main and it will deal with ensuring displayUnits are made to be unique when they come from Slave/Master Aux servers, all meta will have to also route through the Master Main to ensure naming consistency (referencing the unique conversion). So having been through all these thoughts we came to allowing each server to have conversions defined according to its designers wishes. It then communicates everything required to do the conversion (factor etc) in one place in meta to whatever consumers (including upstream servers) are subscribed. Simple, self contained, reliable, but admittedly involving some repetition although only within infrequently transported meta. |
I would propose to make that a server implementation thing. In my post I propose to do a global list of displayUnits, which have to be created prior to a client being able to set them as default for a certain value. SI units to common maritime units could be included by default (i.e. by the entity maintaining the server). The UI used to created global display units, could also include UI to populate the DB for that API. In your example, the One could ask the question: what's the upside of having that API vs putting the conversion along with the created displayUnit in the tree? My only objection tot that is that it would create a massive amount of metadata - especially for more complex conversions - which may not be used by most clients. |
I believe so, yes, because without that I don't see the value of this feature at all. Consider this use case: a company builds SK-powered instrument clusters (along the lines of Garmin GNX/Sailmon/TackTick). A powerful feature would be to set them up centrally. If I make a change using a instrument at the helm, or a phone, I'd like to see that change happen instantly on each screen. |
Sure: a change to the displayUnit of a path would get propagated with meta deltas. I was talking about changes to the conversions themselves: suddenly the parameters of a conversion that was already in the conversions list will change. This is one problem with working on just verbal level: there have already been several misunderstadings in this issue. Writing some code that exercises the features here would demonstrate things more accurately. Sorry, it seems I have trouble making myself understood correctly. |
I don't think naming is a real issue, in these situations. In my opinion, naming conflicts could be solved easily with a sensible default (conflict between master <-> slave, master wins, slave <-> slave whoever registers with master first wins, as it's unit become part of master) and a UI for users to change these conflicts for specific displayUnits (e.g. if there's a SK-enabled engine, I may choose it's units for all my engine related stuff by setting the master server to give it's units priority). Off-topic: in fact, the problem you describe there is true for every resource (regular values are solved by the multiple sources thing) that is the same, within a single context. Let's say there are two slaves that provide a chart for the same area, do we make them both available as multiple "sources"? Doesn't make sense to me - it makes way more sense to treat resources as unique which can't be "sourced" and the actual values as having multiple sources. |
@fabdrol I have approached this is setting the vessel defaults centrally at the server. Not set from individual consumers. To have that functionality is another level of complexity because we need to identify which consumers will use the newly selected units. Imagine multiple personal devices on a vessel with vessel defaults set. If one personal user changes their units you would not expect it to update everyone else's. On the other hand changing the vessel default on the vessel server should chnage any unit which is using vessel defaults. |
I think the same use case applies here: let's say I had a custom unit and there's an error in the calculation, I'd like to be able to change that with immediate effect (in my case, on my Viking warship, I may want to measure oar movements per minute and I'll need to change that on the fly when I replace any of my oarsmen, due to different avg arm lengths and physical strength).
I agree with that, also because that is the whole goal of the node server: a reference implementation, for others to follow. If this is a function of the spec, it should work. |
@bkp7 that's more a permissions thing: which user is allowed to change the server defaults. Related, but out-of-scope IMHO. The starting point is that a consumer uses the default, or chooses a local conversion, or sets a specific default. That last one must at some point be subject to the users' access level (similar to how we wouldn't want every user to be able to change the auto pilot controls). |
So how do you support the use case where I would like to move the units configuration logic from InstrumentPanel code to the server, so that IP can present the popups where you configure the unit that the user want How and where does IP get the lists of conversions? This is an important use case imho, not just propagation of individual display preferences. |
Is this true? It is not currently a reference implementation in that it isn't (and as far as I can see never has been) specification compliant. It also has lots of additional complexity via plugins many of which are also non compliant. I would be very supportive of a reference server implementation which is known to be compliant. |
OK that's fine, I think we are in agreement here in that the vessel default is at the server.
I think this is covered by sending the revised displayUnit property with relevant factor etc. to update the server setting, which is then propagated as the new vessel default? |
No. I did not ask how the chosen displayUnits get propagated, that works just fine. I asked where does IP get the list of conversions so that it can present the popups where you configure the unit that the user wants. |
You are quite right about the node server being a mixture of a reference implementation wannabe and useful extensions. So far it has seemed a valuable approach, but of course I have a veeery heavy bias. I think the best way forward would be to create the set of compliance integration tests. Then the part that is the open source reference implementation functionality would be distinguishable from everything else. This would be something that is valuable to everybody involved. Writing a reference implementation that contains just what's in the spec from scratch seems unrealistic - or there are development resources hiding someplace. For example the conversions from NMEA formats are not part of the spec, but a server without them would not be very useful, and something that is not used will grow stale. |
@tkurki, I in no way meant to demean the work done on node-server. I think we may need a reference server anyway in order to act as a test server that we can spin up to run tests on the test suite. I will look at what would be required for that in terms of resource. Putting that aside and returning to your point about seeing an implementation of this PR, there is still the question of what type of implementation you would want to see on a server. This PR allows the server to be implemented in many different ways and my concern would be that we end up repeating the discussion in #130 when this PR is designed to allow any of those implementation approaches to be taken.
This PR assumes the consumer has a built in set of conversions so the list would be built in and it can push one of those to the server. Actually once we have functionality to update server settings (separate issue) this means a server can propagate a conversion that it was not originally programmed with. |
No offense taken, as I said you are quite right and fresh insight is welcome. |
Just speed read the thread so I might be a bit off mark. What this proposes is that there should be a method to set global defaults on a per vessel basis. The proposal talks about meta for UI's but in fact its about signalk internationalization taken to the vessel or even key level. If we implement this as a I think we need a signalk structure for international 'translations', based on a hierarchical tree similar to the java ResourceBundles concept. Basically we create a tree of properties with increasing specialization. The top level is Locale, eg The tree is recursively read from bottom up, so if I dont specify I can then specialize that to This has the extensibility of adding new nodes to the tree It also allows for current or future UI's that dont do internationalization, and only requires a single key to be stored as server or ui locale preference. Even that can be derived by OS defaults. A first class |
This means |
@rob42 this PR is to enable the setting of vessel wide displayUnits. I mentioned locale because we already have displayName, etc. within meta which represents a user preference as opposed to other data held in meta. Locale could also be a basis for an initial set of displayUnit defaults. The selection of displayUnits is not however directly linked to locale ie. EN.GB does not tell you whether depth should be in meters, feet or fathoms. I do think there is a language issue that could be addressed for example all the enums are in English and each alert message can only be propagated in 1 language, but it is IMO a separate issue. |
Thats where the hierarchy comes in. EN.GB may specify depth in fathoms, so just add EN.GB.modern and override it with meters. Or EN.GB.modern.myboat where I set depth to km |
OK, so I would lookup
I'm struggling to see how this approach could work unless we have a definitive list of conversions baked into the specification? |
@rob42 I don't think it makes sense to have a discussion here about another potential solution for this, the place for that would be in the issue - not in the PR. The point of discussion here is wether this PR is a good solution for the issue at hand. Do you feel that it's not and we should reconsider the whole solution? IMHO whilst no single solution may be perfect, in the sense that it covers every possible scenario and/or is easy to implement, I do feel that the proposed solution fits with the conventions of how we've been doing things in the past. To get a move on with this PR however, I think it's time to make up our mind. Let's do this middle ground: Specification
Implementation
Let's worry about changes later, in a separate issue + PR. That we keep thing nice and scoped. |
@fabdrol I favour reconsidering the solution. I think this PR will only add features that will need to be changed again later as we will have to address full internationalisation anyway, and that can provide a better solution. Yes the discussion should be back in the issue thread - sorry - brain fade... @bkp7 (Thinking about recursive lookup this will be easier to parse) The conversion between units is a separate resource. We should provide common conversions, and a way to add custom conversions. If no conversion can be found we recurse to |
If we want to venture into localisation here beyond simple displayUnits preference, we should leverage HTTP Accept-Language. It would work for http and ws. For background read https://www.w3.org/International/questions/qa-accept-lang-locales If the client sends an Accept-Language header the server could take that into account when crafting the responses. The original proposal in this PR would work ok, the server deciding what to put into The extensions like As the article outlines Accept-Language is not total panacea. The response might depend also on a "session" setting (I want to use now French units, but I am not logged in so it is not a user preference nor a vessel preference) or a user preference. These can be left up to the implementation. If we decide to go this way we can provide localised schemas, where the descriptions are not just english. But as far as I can see this is orthogonal how the actual units preference and conversion parameters are presented in the data model. I firmly believe that we should cater not only for the preference per key but address also
It seems @bkp7 that you steadfastly ignore these use cases, that I listed some days ago. I interpret your comments that you do not want to support any of this, just per path preferences. I believe we should. As for @fabdrol 's suggestion on how to move forward with the implementation: it does not have to be IP, any even brutally simple client demonstrating this functionality would do. |
Good points @tkurki re localisation, maybe copy that + @rob42's ideas into an issue, so we can discuss that later. As for this PR, @bkp7 let me know how you would like to move forward, taking my and Teppo's comments into account. If you feel that that is not the right way to go, I'll close the issue due to lack of consensus and we can continue discussing a better solution in the issue. |
Everyone, thanks for the input I know it all takes up time. I think we need to bring this to a conclusion. Taking everything on board this is where I am: Therefore I think the approach to this issue should be to implement as resources in a similar way to how Waypoints and Routes are already done. Therefore I am closing this PR, and will set out what I mean by this back in the issue. Just for completeness in case we refer back, @tkurki I thought I did address those issues you raised here and here - "My proposal gives: The list is populated with the vessel default display units for that leaf + local units provided by the client". So I was talking about being able to set global units for all paths at the server and I think, reading it again, you envisage setting global units for all paths (including as yet unknown future paths) from the display. And for the list of conversions, I was envisaging a list sufficient to set the display units at the display, whereas you envisage a list including all possible conversions on the server as well, so you can change the vessel defaults from the display (rather than only at the server). We should consider these 2 objectives in PR mk2 |
Thanks @bkp7 |
Add displayUnits to schema. Include samples and modify documentation.
closes #464