fix(timer): BlynkTimer Broken on Low Memory Ports #6

BradenM · 2019-06-20T04:25:10Z

Fixes #5

Notes

On most low memory ports (such as the esp8266) the config variable MICROPY_PY_FUNCTION_ATTRS is disabled, effectively meaning func.__name__ doesn't exist. (See micropython/micropython#614 for more details)

Because Timer relies on func.__name__ to name its timers, its effectively broken on any port without MICROPY_PY_FUNCTION_ATTRS enabled.

Changes

This PR fixes this issue by doing the following:

Checks if func.__name__ exists
If yes, continues as normal
If not:
- Check if obj.func exists and reattempt with that (original fallback)
- If not, then simply return "timer"
Also adds a note in TIMERS.md referring to this change.

This essentially means that on ports without MICROPY_PY_FUNCTION_ATTRS, timers are named X_timer, with X as the original number of timer.

On low memory ports MICROPY_PY_FUNCTION_ATTRS is disabled, leading BlynkTimer to raise an AttributeError when trying to get a name for a new timer. This fixes it by naming each timer 'X_timer' (where X is the # of timer) if the __name__ attribute does not exist. Fixes blynkkk#5

blynktimer.py

antohaUa

Thanks a lot for your findings . All looks good. I have only one small proposal for code modification

antohaUa · 2019-06-20T08:36:59Z

getattr should capture AttributeError if default value was provided.
https://docs.python.org/3.3/library/functions.html#getattr
But i understood what do you mean. Correct if we have recursion and no obj.func then we will fail.
Cause we call it without getattr -> " self._get_func_name(obj.func)"
Thus we can try to change order of verification to avoid such issue:

def _get_func_name(self, obj):
    if getattr(obj, 'func', None) is None:
        return 'timer'
    elif getattr(obj, '__name__', None) is None:
        return self._get_func_name(obj.func)
    return obj.__name__

BradenM · 2019-06-20T08:54:06Z

Okay, I pushed a cleaner version that should work for all cases.
But still,

def _get_func_name(self, obj):
    if getattr(obj, 'func', None) is None:
        return 'timer'
    elif getattr(obj, 'name', None) is None:
        return self._get_func_name(obj.func)
    return obj.name

would also be problematic. function never has a func attribute, so anytime you called that function with a function object (even if obj.__name__ exists) you would receive timer as the return value. And yes I was referring to the AttributeError that would be raised when return self._get_func_name(obj.func) was called, cause getattr will only catch the AttributeError that it raises itself.

Also getattr(object, 'attr', None) is None is the same as hasattr(object, 'attr'), since None is falsy.

To be honest, I just don't understand why obj.func needs to be checked for anyways. I very well could be overlooking it, but I don't see where any object could be passed to _get_func_name that could have a func attribute.

antohaUa · 2019-06-20T09:05:46Z

I ok with hasattr - maybe previously I just get used to getattrs )).
About obj.func - Here is some investigation is needed. For now lets just extend code with additional logic
I think for now without additional investigation it will be more "safety" )

def _get_func_name(self, obj):
    if hasattr(obj, 'func') is False:
        return 'timer'
    elif hasattr(obj, '__name__') is False:
        return self._get_func_name(obj.func)
    return obj.__name__

BradenM · 2019-06-20T09:17:16Z

Well haha, see my co 8000 mment

This will still always return 'timer' for any function object, cause a function object in micropython can only possibly have __class__ or __name__ has base attributes

BradenM · 2019-06-20T09:22:17Z

With:

def _get_func_name(self, obj):
    if hasattr(obj, 'func'):
       return self._get_func_name(obj.func)
    return getattr(obj, '__name__', 'timer')

The flow can go three ways:

1.) obj does indeed have a func attribute

self._get_func_name(obj.func) is called

2.) obj does not have a func attribute, but does have a __name__ attribute

getattr(obj, '__name__', 'timer') will return the functions name

3.) obj does not have a func attribute, nor does it have a __name__ attribute

getattr(obj, '__name__', 'timer') will return 'timer' cause obj does not have a __name__ attribute

Is there another usage case that I am missing?

BradenM · 2019-06-20T09:44:59Z

Okay, final comment for the night haha.

main.py

def _get_func_name(obj):
    if hasattr(obj, 'func'):
       print(obj.func)
       return _get_func_name(obj.func)
    return getattr(obj, '__name__', 'timer')

def test():
    pass

class FakeFuncAttr:
    func = test
    def __init__():
        pass

def test_func_name():
    print("GET FUNC NAME")
    x = test
    print(_get_func_name(x))


def test_with_func_attr():
    print("TEST WITH FUNC ATTR (forced for test)")
    x = FakeFuncAttr
    # setattr(x, 'func', y) - doesnt work on micropython :(
    print(_get_func_name(x))

Results on ESP8266 (no function attrs enabled):

MicroPython v1.11 on 2019-06-03; ESP module with ESP8266
Type "help()" for more information.
>>> 
>>> import main
>>> main.test_func_name()
GET FUNC NAME
timer
>>> main.test_with_func_attr()
TEST WITH FUNC ATTR (forced for test)
<function test at 0x3ffefff0>
timer
>>>

Results on ESP32 (function attrs enabled):

MicroPython v1.11-47-g1a51fc9dd on 2019-06-18; ESP32 module with ESP32Type "help()" for more information.
>>> 
>>> import main
>>> main.test_
test_func_name  test_with_func_attr
>>> main.test_func_name()
GET FUNC NAME
test
>>> main.test_with_func_attr()
TEST WITH FUNC ATTR (forced for test)
<function test at 0x3ffc3b00>
test
>>>

Is this not what we are trying to achieve? Hope we come to a conclusion here haha, but I am gonna catch some sleep. Its been fun haha.

antohaUa · 2019-06-20T12:01:47Z

Finally I have remebered purpose of obj.func ))
It is used when we have constructions like

@timer.register(interval=2)
@ti
8000
mer.register(interval=1)
def test():
    print('Hello!')

Initially I on quick view I was thinking that this is something internal )

So here you are right and you solution will cover expected cases.
Let's just add comments for quick understanding what goes on for future cases:

def _get_func_name(self, obj):
       # double decorator case
       if hasattr(obj, 'func'):
           return self._get_func_name(obj.func)

       # default 'timer' value to support micropython low memory ports
       return getattr(obj, '__name__', 'timer')

BradenM · 2019-06-20T15:14:20Z

Haha! Wow, thats it, I never thought about that possibility. But yeah your definitely right about adding some comments. Went ahead and updated it.

antohaUa · 2019-06-20T19:36:42Z

Thx!

BradenM added 2 commits June 19, 2019 23:08

docs(timer): Added note about Low Memory ports

ab94218

antohaUa reviewed Jun 20, 2019

View reviewed changes

blynktimer.py Show resolved Hide resolved

antohaUa reviewed Jun 20, 2019

View reviewed changes

feat(timer): Cleanup Code, handle all cases

e7b66dc

docs(timer): Added some docstrings for clarification

bb9f09a

antohaUa merged commit 37df411 into blynkkk:master Jun 20, 2019

BradenM deleted the fix/low-mem-timer branch June 20, 2019 19:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(timer): BlynkTimer Broken on Low Memory Ports #6

fix(timer): BlynkTimer Broken on Low Memory Ports #6

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fix(timer): BlynkTimer Broken on Low Memory Ports #6

fix(timer): BlynkTimer Broken on Low Memory Ports #6

Uh oh!

Conversation

Uh oh!

Notes

Changes

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!