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

Add a QT GUI #82

Merged
merged 1 commit into from
Nov 20, 2020
Merged

Add a QT GUI #82

merged 1 commit into from
Nov 20, 2020

Conversation

chrisveilleux
Copy link
Member

Description

Add the QML and hooks into the GUI framework necessary to display timers on a Mark II

Type of PR

If your PR fits more than one category, there is a high chance you should submit more than one PR. Please consider this carefully before opening the PR.
Either delete those that do not apply, or add an x between the square brackets like so: - [x]

  • Bugfix
  • Feature implementation
  • Refactor of code (without functional changes)
  • Documentation improvements
  • Test improvements

Testing

Load onto a device using the QT GUI framework and set timers.

Documentation

Documentation on the skill already exists.

@JarbasAl
Copy link

JarbasAl commented Nov 21, 2020

why is this mark2 only? doesnt it work in all GUIs?

the enclosure check is dumb, if anything check for mark1 which only supports the default. Why does everyone get the mark1 render which only works in 1 device, but the GUI that works everywhere checks for mark2?

GUI can even be remote and connected to a mark1, so i would argue the enclosure check is non sense, might aswel call both functions since they don't conflict i think. In the end the mark1 needs to be abstracted away in a next step, or you will end up with lots of kludges all over the place

@forslund
Copy link
Collaborator

Agree with Jarbas, just publish the events and if a gui is connected it'd be shown if no gui is connected it won't. I don't think it's needed but there is also the possibility to use self.gui.connected to verify. But if might be as well to always populate the gui so the page is shown if a gui is connected.

@krisgesling
Copy link
Contributor

Note to anyone following: this PR got merged unintentionally.

This is looking good.

I think it makes sense to generalize the GUI to all platforms.

I also wonder if we can have a single render_timer function that either calls render_qt_gui, or refactor a little to something like:

def render_timer(self, idx, seconds, timer=None):
    self.render_simple_timer(idx, seconds) # existing render code
    if self.gui.connected:
        self.render_gui_timer(...)

In terms of the countdown logic, I'm still of the opinion that this would be better handled on the qml side. The inconsistent time changes are very noticeable.

Not for this iteration:

  • With the progress bar, for short timers (eg 10 seconds) the bar will jump forward on each second. We could animate this to be a smooth fill.
  • Still haven't figured out the best way to handle multiple timers reusing the same qml page rather than duplicating it...

@AIIX
Copy link
Contributor

AIIX commented Nov 23, 2020

* Still haven't figured out the best way to handle multiple timers reusing the same qml page rather than duplicating it...

One can send the timer data as a model instead of single timer data as in:

Instead of:

  self.gui['timer_color'] = BACKGROUND_COLORS[color_idx]
  self.gui['timer_name'] = timer_name
  self.gui['percent_elapsed'] = percent_elapsed
  self.gui['time_remaining'] = remaining_time_display

One could use a model like below and append a new timer to it once a new timer is created:

self.gui['timer_data'] = [{"timer_color": BACKGROUND_COLORS[color_idx], "timer_name": timer_name, "percent_elaspsed": percent_elapsed, "time_remaining": remaining_time_display, "timer_id": 1}, {"timer_color": BACKGROUND_COLORS[color_idx], "timer_name": timer_name, "percent_elaspsed": percent_elapsed, "time_remaining": remaining_time_display, "timer_id": 2}, {"timer_color": BACKGROUND_COLORS[color_idx], "timer_name": timer_name, "percent_elaspsed": percent_elapsed, "time_remaining": remaining_time_display, "timer_id": 3}] 

And then use a listview in QML to use the same qml of the timer to replicate itself based on the model data.

@AIIX
Copy link
Contributor

AIIX commented Nov 23, 2020

In terms of the countdown logic, I'm still of the opinion that this would be better handled on the qml side. The inconsistent time changes are very noticeable.

it seems gui api core side does not implement "mycroft.session.list.update" https://github.com/MycroftAI/mycroft-gui/blob/master/transportProtocol.md#updates-item-values-starting-at-the-given-position-as-many-items-as-there-are-in-the-array which might be why there is inconsistent time changes, as one might just be reinserting a new model instead of updating values of an existing model

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants