Set RunLoop up for re-entrant usage

[furbrain]

This PR allows my GUI code to start it's own iteration of RunLoop.run to allow for dialogs to run "modally" while still keeping events specified by every and once to run as normal. I've also added some code to simplify cancelling an every timer from within that timer. Also small bug-fix in graphics module

[joerick]

Hey @furbrain, thanks for this. I'm wondering if we can do the remove_timer and reentrant run stuff without the instance variables - it feels fragile to me. Do you mind if I do a bit of work on top of your commits?

[furbrain]

Feel free. I agree that it seems a bit over-engineered.

On 17 June 2016 at 10:29, Joe Rickerby notifications@github.com wrote:

Hey @furbrain https://github.com/furbrain, thanks for this. I'm wondering if we can do the remove_timer and reentrant run stuff without the instance variables - it feels fragile to me. Do you mind if I do a bit of work on top of your commits?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/tingbot/tingbot-python/pull/31#issuecomment-226723922, or mute the thread https://github.com/notifications/unsubscribe/AC233fkIPJhorKmgQU6cAj_loazf2ORaks5qMmjhgaJpZM4I33cB .

[furbrain]

We could enable access to an every() function (separate from the decorator), which returns a handle on the timer object, which could then be used to mark it as stopped. I'd need to change my gui code, but that's no biggie. The more I think about this, the more this is the right way to go. It's a bit neater than how I've done this.

Phil

On 17 June 2016 at 10:32, Phil Underwood beardydoc@gmail.com wrote:

Feel free. I agree that it seems a bit over-engineered.

On 17 June 2016 at 10:29, Joe Rickerby notifications@github.com wrote:

Hey @furbrain https://github.com/furbrain, thanks for this. I'm wondering if we can do the remove_timer and reentrant run stuff without the instance variables - it feels fragile to me. Do you mind if I do a bit of work on top of your commits?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/tingbot/tingbot-python/pull/31#issuecomment-226723922, or mute the thread https://github.com/notifications/unsubscribe/AC233fkIPJhorKmgQU6cAj_loazf2ORaks5qMmjhgaJpZM4I33cB .

[joerick]

Agreed, I think exposing the real Timer objects would be an improvement.

[joerick]

Here's what I'm thinking as an API to run loop reentrancy...

import tingbot
from tingbot import *

@left_button.press
def press():
    def modal_draw():
        screen.fill(color='black')
        screen.text('[modal]\nwaiting for touch...')

    touch_detected = False

    def detect_touch():
        touch_detected = True

    modal_draw_timer = Timer(action=modal_draw, seconds=1.0/30, repeating=True)

    tingbot.main_run_loop.add_timer(modal_draw_timer, modes=['modal'])
    tingbot.input.push_touch_handler(detect_touch)

    tingbot.main_run_loop.run_until(lambda: touch_detected, mode='modal')

    tingbot.input.pop_touch_handler()
    tingbot.main_run_loop.remove_timer(modal_draw_timer)

    screen.fill(color='black')
    screen.text('[modal finished]')

@every(seconds=1.0/30)
def draw():
    screen.fill(color='black')
    screen.text('[main]\ndoo be doo be doo...')

tingbot.run()

All timers are in the 'normal' mode by default. The touch handler will register to be in both the 'normal' and 'modal' modes. By running the run loop in a 'modal' mode, we can deactivate the main draw() callback (and button callbacks) while the modal is displayed, and reenable when we return to the normal mode.

I've not implemented anything yet, but this is what I'm thinking. What do you think?

[furbrain]

I'm not sure about having named modes for the run_loop. The issue is that you may have one dialog, which then spawns another one; keeping track of different names would be challenging. I think it may be better to have a LIFO queue of run_loops, and specify when setting up a timer whether it should be specific to the current run_loop or whether it should run all the time.

[furbrain]

I'll make some changes to demonstrate what I am meaning...

[furbrain]

Also I think that the remove_timer should be an attribute of the timer, not the run_loop. You may have two timers set for the same function, and only want to remove one of them

[joerick]

A run loop stack did cross my mind, as a more general solution. But perhaps a navigation stack could better be supported by a proper GUI library, rather than this run loop tinkering. The 'modal' mode just being for blocking instantiation password = keyboard_prompt(prompt='Password', initial='qwerty123').

[joerick]

I pushed a few commits of the reentrant run loop modifications to https://github.com/tingbot/tingbot-python/tree/runloopreentrant

Not done anything re. modes/stack yet!

[furbrain]

I've pushed some changes to this PR - implements a run_loop stack which is managed by the RunLoop class using some classmethods. This feels a lot more stable than my previous iteration. This works nicely with the GUI design and would allow for nested blocking modal calls - e.g. with my keyboard implementation it would be useful to have the emoji button bring up a further sub-dialog

[furbrain]

Moving the remove_timer feature into Timer has actually removed most of the re-entrancy problems with the run_loop.main

[joerick]

I'm coming round to the stack idea! But instead of keeping a stack in a RunLoop object, why not use the call stack itself? So create a new RunLoop object wherever you need a restricted run environment for e.g. a modal.

The way your code inherits before/after/wait callbacks isn't isolated enough, I think. This will call users' button handlers which could draw over our model dialog. Creating a new RunLoop instance and explicitly registering what we want it to do that solves the problem, I think. We can make a nice API for this (see run_loop.register below)

import tingbot
from tingbot import *

@left_button.press
def press():
    def modal_draw():
        screen.fill(color='black')
        screen.text('[modal]\nwaiting for touch...')

    touch_detected = False

    def detect_touch():
        touch_detected = True

    modal_draw_timer = Timer(action=modal_draw, seconds=1.0/30, repeating=True)

    # create a run loop just for this modal dialog
    run_loop = RunLoop()

    # we want to use the screen, and receive touch events, so let's plug those in to the run loop
    run_loop.register(screen, tingbot.input)

    run_loop.add_timer(modal_draw_timer)
    tingbot.input.push_touch_handler(detect_touch)

    run_loop.run_until(lambda: touch_detected)

    tingbot.input.pop_touch_handler()

    screen.fill(color='black')
    screen.text('[modal finished]')

@every(seconds=1.0/30)
def draw():
    screen.fill(color='black')
    screen.text('[main]\ndoo be doo be doo...')

tingbot.run()

[furbrain]

Sorry for the delay getting back to this - been kind of busy. Using a register function looks interesting, but I'm not sure how it would know to connect the screen update function to the wait queue and the after_action queue, whereas input just goes in the wait queue (which is what currently happens).

[furbrain]

Also if we have a timer that we want to run all the time we could do with some way of marking that timer as persistent for this run_loop and any run_loops that would be spawned from this level of run_loop. That would require access to the current running run_loop - which could be done with the stack built in to RunLoop class

[furbrain]

Not sure how to address the button press issue

[joerick]

Hey @furbrain, no worries at all. I've been busy with manufacturing stuff :)

how it would know to connect the screen update function to the wait queue and the after_action queue, whereas input just goes in the wait queue

I'd imagine that there's a protocol for modules that can attach to the run loop, probably a function that the run loop would call in register e.g. module.attach_to_run_loop(run_loop), and the module itself would decide what it needed to attach.

So example RunLoop.register(module):

def register(self, module):
    module.attach_to_run_loop(self)

Screen.attach_to_run_loop:

def attach_to_run_loop(self, run_loop):
    run_loop.add_after_action_callback(self.update_if_needed)
    run_loop.add_wait_callback(self.update_if_needed)

Also if we have a timer that we want to run all the time we could do with some way of marking that timer as persistent for this run_loop

I'd have to see a use-case for this, but perhaps a background thread would be better suited? I'm not really expecting modals to be used much, only when the user is directly interacting.

[furbrain]

I'm not sure having separate register functions for a module are necessary - it's starting to feel a bit overengineered, especially when it's only likely to ever be called from two places - __init__.py in tingbot-python and dialogs.py in tingbot-gui. I've fixed the buttons issue by only copying over the timers from the parent RunLoop, and then adding in a separate event handler for touch events and plugging in access to the screen within RunLoop.__init__- I can't foresee an instance where we would want a runloop that does not update the screen if needed.

These changes also mean that the pop/push_event_handler code now becomes redundant. What do you think?

[furbrain]

Ok, have struck out the background attribute for Timers and the stack attribute for RunLoop. This should make the whole code a lot cleaner

[joerick]

Thanks @furbrain, this is in a good state now!