Gpiozero: Consider reimplementing blink/pulse by setting source

[bennuttall]

As suggested in #449, perhaps blink and pulse should be implemented by setting .source rather than in a blink thread.

Also consider suggestion #397 - adding is_blinking property.

[lurch]

Would we _really_ need a .is_blinking property, or would it be sufficient to just check .source != None ?
Otherwise would we also get requests for .is_pulsing or .is_fading properties?

[bennuttall]

@martinohanlon what do you think?

[martinohanlon]

I like the idea of using source internally to manage changes in value (i.e. blinking, fading), its self contained and can be implemented consistently across components.

I dont think an .is_blinking property is needed, it feels very specific with little reuse and as @lurch says where do you stop... If a user really needed to keep track of whether a component was blinking its an easy implementation in the calling program.

[lurch]

I dont think an .is_blinking property is needed

Should #397 be closed then?

[bennuttall]

I'm happy with that :+1:

[martinohanlon]

Let no one say I am not prepared to re-evaluate my views... ;)

[normanr]

Actually I was thinking of writing a helper (in the form of a decorator), that you could add to a generator function that would allow you to introspect the source function once the generator has started.

def smart_generator(fn):

  class inner(object):

    def __init__(self, *args, **kwargs):
      self.func = wrapper
      self.args = args
      self.kwargs = kwargs
      self.thread = None

    def __iter__(self):
      assert self.thread is None, 'smart_generator already iterating'
      self.thread = threading.current_thread()
      for v in fn(*self.args, **self.kwargs):
        yield v
      self.thread = None

    def __repr__(self):
      return '<smart_generator %s(%s) at 0x%08x on %s>' % (
        fn.__name__,
        ', '.join(
          ([', '.join(repr(a) for a in self.args)] if self.args else []) +
          ([', '.join('%s=%r' % i for i in self.kwargs.items())] if self.kwargs else [])),
        id(self),
        self.thread)

  @functools.wraps(fn)
  def wrapper(*args, **kwargs):
    return inner(*args, **kwargs)

  return wrapper

@smart_generator
def foo(a, b, d=8, e=9, c=7):
  'does things'
  for x in range(5):
    yield x

f = foo(1, '2', c='3', d=4)

print(f)  # <smart_generator foo(1, '2', c='3', d=4) at 0xf78d9a8c>
assert f.func == foo

(latest copy at http://www.darkskies.za.net/~norman/scripts/gentest.py)

Then this allows a few things:

1. You can tell if an led is blinking by checking led.source.func == blinker (assuming blinker is a generator function like those in tools)
2. You can print(led.source) and get useful output describing what the source is doing, if it's still running, and on what thread. (edit: I updated the code above to implement current thread storage mentioned in the next comment)
3. You could only use source_delay if source is directly set to ValuesMixin.values, any other generator would need to be wrapped in something like tools.post_delayed.

[normanr]

It many also be required to store current_thread on the smart_generator when it's active otherwise when _copy_values tries to set value, then it'll probably stop itself. The value setter shouldn't stop the thread if it's the thread currently iterating and setting the values.

[normanr]

Actually the more I think about this the original generator function should just be implemented as an iterable class (this would also allow parameters to be changed without replacing the event source - allowing for smoother transitions) The decorator is too much python magic.