Teppo Kurki - @santino1919 would be interested ...

@santino1919 would be interested in the design of metadata editing in the server UI? There are two main use cases: - adding units and other misc textual fields to paths not in the specification - configuring zones: nominal/warn/emergency value ranges for a path
31 Replies
Scott Bender
Scott Bender3mo ago
What about “defaults”? Could be there too?
David Godin
David Godin3mo ago
We should move them back to base deltas no ?
Scott Bender
Scott Bender3mo ago
They are there. Don’t need moved. We just don’t have a way to edit arbitrary data there Not sure if that should be a part of the meta data editor.
Teppo Kurki
Teppo Kurki3mo ago
baseDeltas is the (current) storage format, we need a UI
David Godin
David Godin3mo ago
We need a good UX to drive adoption Sorry. What I meant was should meta plugins save and write to baseDeltas
Teppo Kurki
Teppo Kurki3mo ago
If we have a UI for editing and the data is available over the apis then baseDeltas or a new separate file or something else does not really matter
David Godin
David Godin3mo ago
From what I know, it’s not much, baseDeltas stores vessel setting already, and you can add properties as per meta specs and it will process it properly. All keys, including zones. Maybe my memory is failing but back when I built a full meta editor, it was all working fine. We should centralize there and not have multiple places where meta can be stored. No? I had to look in a couple of places while testing new Edit Zone for that reason. I annoyingly reiterate my idea: we should expose baseDeltas to openAPI with some kind of CRUD and build the meta editor on top.
Teppo Kurki
Teppo Kurki3mo ago
so if I want to set the longName of propulsion/styyra/revolutions to Styyra kierrosluku what kind of an API call should I make if the api is built on baseDelats? baseDeltas is an array of deltas. Do I need to first retrieve all the deltas so that I can check that if there is already a meta delta for this path? can I replace the nt'h delta? what if that delta has also a value for shortName, do i need to send that also? or do i need to send always all the deltas? what if another user (or I myself in another window) modifies the baseDeltas between this window retrieving the previous values and saving them? to me baseDeltas is not a very good abstraction for a CRUD api a rather obvious api would be a PUT to propulsion/styyra/revolutions/meta/longName, then the server implementation can do what is needed to persist that and send it to clients I am definitely not saying that there should be multiple places to store metadata, just that the API should be built based on the API use cases, not how the server happens to function internally today
David Godin
David Godin3mo ago
What I mean by CRUD is just a way to retrieve and set meta. @Scott Bender @AdrianP. Let try and see where we agree. I'm asking because I'm rewriting meta/zones in KIP and if it's not settled, I'll keep the local logic implementation. We agree that: 1- The meta structure should be whats in the spec 2- No need to store in different places so why not use baseDeltas. It's processed by the server already. Am I right @Scott Bender ? 3- We need a good UX in SK. 4- While we are at it, we should do all spec keys and not just Zones. KIP will for sure exploit them. What does not work for you? I don't understand.
Teppo Kurki
Teppo Kurki3mo ago
Store in baseDeltas: yes Structure the api around baseDeltas: no
David Godin
David Godin3mo ago
You mean you don’t like the meta specs?
Teppo Kurki
Teppo Kurki3mo ago
Please read my message above about how the api should work. Could we move on to that? How do you see the api should work to create, update and delete metadata?
Scott Bender
Scott Bender3mo ago
We have PUT for meta data already. We should just add DELETE?
David Godin
David Godin3mo ago
I like that. It was the only problem I hit way back when. I'm not really stuck on the technical, API or not, aspects per say. I mentioned CRUD just as a "concept" to illustrate what a Client will need. I think with Scott's proposal, we should be good. I had tried to PUT null/undefined/[] against some key but it broke the json. Is this something someone can look at? It sounds like a strong understanding of meta schema and SK PUT process is necessary. I can take the client side DELETE implementation and validation. I have a draft ready.
Teppo Kurki
Teppo Kurki2mo ago
the server already supports PUT, @Scott Bender has a wip implementation of DELETE and I created an initial implementation of metadata editing in the Server UI
GitHub
feature: add support to DELETE meta data by sbender9 · Pull Request...
There is one issue... When there is the same meta data from baseDeltas and from another source (like a plugin). This will delete the plugins meta for that field until restart or the plugin sends it...
GitHub
[WIP] Metadata editing by tkurki · Pull Request #1730 · SignalK/sig...
Add support for editing metadata. The goal of this PR is to add the functionality, not be pretty - let's get the initial end2end functionality out there. Todo: state handling & save add ...
David Godin
David Godin2mo ago
Awesome. Thank you guys! 🥳
Teppo Kurki
Teppo Kurki2mo ago
No description
David Godin
David Godin2mo ago
Once you are ready with the UI and DELETE to meta is released, I’ll use it in KIP gauges. Not sure if/how I’ll build the integration yet, but everything meta will be in sync: scales, label and zones. It will create breaking changes for KIP with those config changes.
Teppo Kurki
Teppo Kurki2mo ago
Ok, I think the basic functionality is there now. It is now building a docker image, once that is finished you can try this out with docker pull signalk/signalk-server:metadata-editing && docker run --init --rm -it -p 3000:3000 -v /tmp/:/home/node/.signalk --entrypoint /usr/lib/node_modules/signalk-server/bin/signalk-server signalk/signalk-server:metadata-editing --sample-nmea0183-data
David Godin
David Godin2mo ago
I know nothing of docker. Guess I need to learn new stuff🤪
Teppo Kurki
Teppo Kurki2mo ago
install and copypaste from above to get started the idea here is that the docker image has the admin ui from the branch prebuilt, so really easy to get it up and running
David Godin
David Godin2mo ago
@Teppo Kurki this is really cool. It replaces VMs basically? @Teppo Kurki nice work! Am I right in thinking we will not need Edit Zones anymore?
Teppo Kurki
Teppo Kurki2mo ago
Yes, the plugin will go away
David Godin
David Godin2mo ago
@Teppo Kurki I don't get notifications after creating Zones.I added Methods too. At first glance baseDeltas looks good
Teppo Kurki
Teppo Kurki2mo ago
As you can see from the code this is just the ui at this point
David Godin
David Godin2mo ago
ah... Did not look at the code. Just got Docker working. That's a big day for me! I tought the server already picked up zones keys from baseDeltas and sent notifications
Teppo Kurki
Teppo Kurki2mo ago
pull again, updated image now triggers notifications
David Godin
David Godin2mo ago
Nice! I think we are missing zone.method
Teppo Kurki
Teppo Kurki2mo ago
We are, will add Pull again, now with method
David Godin
David Godin2mo ago
Nice work. Works great so far! We are only missing displayScale 😉 Is there a way to merge this PR with yours as a docker image? https://github.com/SignalK/signalk-server/pull/1734
Teppo Kurki
Teppo Kurki2mo ago
Yep displayScale needs a custom widget Merging 1734 and rebasing on master would pull that into this PR