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
What about “defaults”? Could be there too?
We should move them back to base deltas no ?
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.
baseDeltas is the (current) storage format, we need a UI
We need a good UX to drive adoption
Sorry. What I meant was should meta plugins save and write to baseDeltas
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
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.
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 todayWhat 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.
Store in baseDeltas: yes
Structure the api around baseDeltas: no
You mean you don’t like the meta specs?
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?
We have PUT for meta data already. We should just add DELETE?
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.
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 ...
Awesome. Thank you guys! 🥳
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.
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-dataI know nothing of docker. Guess I need to learn new stuff🤪
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
@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?
Yes, the plugin will go away
@Teppo Kurki I don't get notifications after creating Zones.I added Methods too.
At first glance baseDeltas looks good
As you can see from the code this is just the ui at this point
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
pull again, updated image now triggers notifications
Nice! I think we are missing zone.method
We are, will add
Pull again, now with method
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
Yep displayScale needs a custom widget
Merging 1734 and rebasing on master would pull that into this PR