Skip to content
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

No instrument update when source changes #176

Closed
JonyIvy opened this issue Apr 4, 2023 · 12 comments · Fixed by #228
Closed

No instrument update when source changes #176

JonyIvy opened this issue Apr 4, 2023 · 12 comments · Fixed by #228
Assignees
Labels
bug A bug report

Comments

@JonyIvy
Copy link

JonyIvy commented Apr 4, 2023

I have two sources for propulsion.main.temperature, one is can0.22, the other can0.37. SignalK's source priorities are set up so that can0.37 has precedence over can0.22. For some reasons, can0.37 can drop out, then can0.22 should take over.

KIP seems to lock in on the first source it finds for the path. If that source stops but the same path is received from the other source, the instruments keep showing the last value from the first source and don't update any more with the data from the new source.

@godind
Copy link
Collaborator

godind commented Apr 4, 2023

Hi,

Thanks for taking the time to send feedback! At the moment, Kip only works with single sources paths. We have no plans to add multiple source support as it is not a commun scenario and there is no quick fix.

You can submit a feature request, or you are welcome to fork Kip source code, add the feature and to submit it for integration to the project.

@godind godind added enhancement An enhancement request help wanted labels Apr 4, 2023
@tkurki
Copy link

tkurki commented Apr 4, 2023

It is your prerogative to choose what issues to tackle and this being open source things are what they are.

But imho this is a bug not an enhancement - from a user's perspective Kip stops updating the value on the screen, when there is newer data received.

I think having multiple GPSs for example is more common than you think and as SK and Kip usage spreads the bug will get more visible. This is why the source data is in delta messages, so that applications can discern input from different sources and choose how to behave, so naturally I feel a little disappointed with the situation.

PS. Why would you want somebody to submit a feature request when you already have this issue?

@JonyIvy
Copy link
Author

JonyIvy commented Apr 4, 2023

I had the feeling KIP did support multiple sources, because you can already select from which source you want the data to be picked up. Leaving that at "default" suggested to me KIP would just take data for that path from any source. So a fix should probably just implement this behaviour.

@mxtommy
Copy link
Owner

mxtommy commented Apr 4, 2023

@tkurki Is there some spec on how to handle multiple sources in UI? (A quick glance I didn't see it). Current behavior in KIP is to set the first source it sees for a path as default. Then User can select another if that's not the correct one. It's been that way since day 1.

In order to failover to another source, would need to implement some kind of preference for sources? (if you have 3, and your primary source fails, which of the other 2 do you display? This would likely be rather involved coding wise. UI to set preferences, updates to logic to use that preference, Do you show anything when failing over from source A to source B, etc...)

If instead we just show any/all values for a path regardless of source, your value could be jumping between two slightly different readings? If I remember the old demo data for demo.signalk.org originally had this for some path which is one of the reasons I forced KIP to pick one.

For example GPS speed might always be a little off each other if you have multiple units. If it's constantly jumping up/down 0.1knots every update, not ideal either. (think with GPS 1 update/second, GPS A sends 3.2 knots. Then 0.2 seconds later GPS B sends 3.1 knots. then 0.8 seconds later repeat.... All user sees is GPS speed flickering from 3.1 to 3.2 for a fraction of a second every second.)

Having an option to display data from "any" source is probably easier coding wise if it makes sense. That said not sure how much priority this feature would have. I've unfortunately been a bit slow on the development without anything to motivate me (anyone want to buy me a sailboat? :D ) and I'm not @godind 's boss either :D

@tkurki
Copy link

tkurki commented Apr 4, 2023

No, there is no spec for UIs. To be honest InstrumentPanel doesn't have this either - widgets/gauges there are strictly per-path-and-source.

But both IP and Kip predate the server's Source Priorities feature, where you can define the priority order for different sources for a path and timeouts, so that if the higher prio source goes silent data from a lower prio is passed through. The feature discards data from lower prio sources if there's fresh enough data from the higher prio source.

I think the confusion here relates to having default in the gauge configuration. That sounds more like any than first, which it really is, if I understood things right. And nowadays, with the server's Priorities, any would make sense.

@godind
Copy link
Collaborator

godind commented Apr 5, 2023

Hey guys. I can see how we would do this but as stated, it's the first time this usecase has been reported.

@tkurki Would be nice to have sk consumer paths handle source and priority for clients.

@tkurki
Copy link

tkurki commented Apr 5, 2023

@godind what do you mean? What is ”consumer paths”? The server does handle source priorities, per my comment above.

@godind
Copy link
Collaborator

godind commented Nov 7, 2023

I'll update the UI to first found. I think it makes more sense but I'll confirm the logic in the code first.

Thanks for the feedback.

@godind godind added bug A bug report and removed enhancement An enhancement request help wanted labels Nov 7, 2023
@godind godind closed this as not planned Won't fix, can't repro, duplicate, stale Nov 11, 2023
@tkurki
Copy link

tkurki commented Nov 11, 2023

I'll update the UI to first found
godind closed this as not planned 3 hours ago

Did I understand this correctly:

  • the user can not specify the source, Kip locks to the first found source for a path
  • if that source goes silent KIP stops updating even if there is another active source
  • Kip does not support multiple gauges for a single path
  • this is not going to be fixed

@tkurki
Copy link

tkurki commented Nov 11, 2023

Ah, i reread the issue. This only affected default source selection, right. And I naturally could check things out myself…

@mxtommy
Copy link
Owner

mxtommy commented Nov 11, 2023

I didn't speak with @godind about the issue, but re-reading the comments I think he just means he will change the UI to show "first found" instead of "default" to make the current existing behavior more obvious.

@godind godind reopened this Nov 22, 2023
@godind
Copy link
Collaborator

godind commented Nov 22, 2023

Reopening after conversations and investigating.

@godind godind self-assigned this Nov 23, 2023
godind added a commit that referenced this issue Nov 23, 2023
godind added a commit that referenced this issue Nov 24, 2023
@godind godind linked a pull request Nov 26, 2023 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A bug report
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants