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

Pushing items into an empty stack on micropyhton esp32 #6

Closed
RaphaelMaschinsen opened this issue Mar 20, 2019 · 5 comments
Closed

Pushing items into an empty stack on micropyhton esp32 #6

RaphaelMaschinsen opened this issue Mar 20, 2019 · 5 comments
Assignees
Labels

Comments

@RaphaelMaschinsen
Copy link

RaphaelMaschinsen commented Mar 20, 2019

Hi,

thanks alot for the effort you put into this great library. It's really awesome you made it available for micropython!

I think there might be a small problem with the way items are added to an empty stack on micropython (esp32):
If i try to make the rpn_calculator.py example work on my esp32 i run into the following problem:

>>> calc.caluculate(' 167 3 2 2 * * * 1 - =')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "<stdin>", line 38, in caluculate
  File "<stdin>", line 35, in parse
  File "/lib/pysm/pysm.py", line 673, in dispatch
  File "<stdin>", line 44, in start_building_number
  File "/lib/pysm/pysm.py", line 329, in push
  File "/lib/pysm/pysm.py", line 55, in append
TypeError: unsupported types for __gt__: 'NoneType', 'int' 

The problem is that:
self.sm.stack.push(int(digit))
in rpn_calculator.py tries to push into an empty stack, initialized like this:

    def __init__(self, maxlen=None):
        self.deque = deque(maxlen=maxlen)

maxlen ends up as NoneType and therefore can't be compared with > in the append() function in the class deque_maxlen:

        def append(self, item):
            if self.maxlen > 0 and len(self.q) >= self.maxlen:
                self.q.popleft()
            self.q.append(item)

And in the rpn_calculator.py file there is a typo i think it should be calculate() and not caluculate()

@RaphaelMaschinsen
Copy link
Author

As a quick fix i added this try ... except ... in pysm.py:

    class deque_maxlen(object):
        def __init__(self, iterable=None, maxlen=0):
            # pylint: disable=no-member
            self.q = deque_module.deque(iterable)
            self.maxlen =  maxlen

        def pop(self):
            return self.q.pop()

        def append(self, item):
            try:
                if self.maxlen > 0 and len(self.q) >= self.maxlen:
                    self.q.popleft()
            except:
                pass

@pgularski
Copy link
Owner

Hi Raphael,

Good finding! Thanks!

The quick fix you're suggesting is silencing the exception but it doesn't fix the problem with pysm.Stack instances that do not specify explicitly the maxlen property. Ie. stack = pysm.Stack() is flawed. The stack basically doesn't work and you're not able to push anything to it.

The Python documentation states that If maxlen is not specified or is None, deques may grow to an arbitrary length.

Therefore, the fix should read something like this:

    class deque_maxlen(object):
        def __init__(self, iterable=None, maxlen=0):
            # pylint: disable=no-member
            if maxlen in [None, 0]:
                maxlen = float('Inf')
            self.q = deque_module.deque(iterable)
            self.maxlen = maxlen

So for a quick fix you can try the above.

In fact I've created a branch with a fix for this bug here with some unit tests but I haven't had a chance to test it on a ESP32 yet.

@pgularski pgularski added the bug label Mar 22, 2019
@pgularski pgularski self-assigned this Mar 22, 2019
@pgularski
Copy link
Owner

With the latest change on branch Issue-6-Pushing-items-into-an-empty-stack-on-micropyhton-esp32 (see the diff) I got the test_rpn.py working with MicroPython v1.10-230-ge0c6dfe90 on 2019-03-22; linux version.
Still didn't test it with ESP32 though.
And the code needs some polishing. But you get the idea.
P.

@RaphaelMaschinsen
Copy link
Author

Thanks a lot for the updates. I just tested test_rpn.py with the updated pysm.py on the esp32 hardware and everything works without a problem now. Also I just wanted to say thanks again for the great project!

@pgularski
Copy link
Owner

Raphael,

Thanks again for raising this issue and submitting a proposed solution.

I fixed the bug and tested it against the following Micropython versions:

  • MicroPython v1.10-230-ge0c6dfe90 on 2019-03-22; linux version
  • MicroPython v1.9.4 on 2018-05-11; ESP32 module with ESP32

I published a new release - 0.3.8-alpha so you can easily use upip to install the fixed version.

P.

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

No branches or pull requests

2 participants