picoweb icon indicating copy to clipboard operation
picoweb copied to clipboard

add_reader/add_writer/remove_writer crash when using not using Pycopy, as otherwise required by README

Open ernstnaezer opened this issue 5 years ago • 16 comments

Hi,

not sure if this belongs in Picoweb or in uasyncio - but when I try to run the example webserver I get the following crashlog:

Traceback (most recent call last):
  File "main.py", line 30, in <module>
  File "/lib/picoweb/__init__.py", line 298, in run
  File "/lib/uasyncio/core.py", line 155, in run_forever
  File "/lib/uasyncio/core.py", line 130, in run_forever
  File "/lib/uasyncio/__init__.py", line 60, in remove_writer
  TypeError: function takes 2 positional arguments but 3 were given

The webserver code isn't anything special, just wait for WiFi and host an app; both a route and a static folder.

I tried upip.install as well as copy & past the latest Picoweb, uasyncio & uasyncio.core from the Github repos.

ernstnaezer avatar Dec 26 '18 15:12 ernstnaezer

The traceback you posted is cut off, and doesn't show what error happens. But likely, you need to use https://github.com/pfalcon/pycopy , as the README suggests.

pfalcon avatar Dec 26 '18 15:12 pfalcon

It's mentioned in this commit - upgrade to your fork as you suggestion. I'll have a look tomorrow and this issue can probably be closed.

https://github.com/pfalcon/micropython-lib/commit/4f70217e4fce6a6ca3c50ea8552a8ec094af0d34#diff-c502b73effb6927190c86fdaab27ee84


The traceback you posted is cut off, and doesn't show what error happens.

Ah yes, sorry! Fixed in the original description. I'll give your suggestion a try to see if that fixes the issue.

Dug a little further and it seems to go wrong on the last line of remove_writer:

    def remove_writer(self, sock):
        if DEBUG and __debug__:
            log.debug("remove_writer(%s)", sock)
        self.objmap.pop(id(sock), None)
        # StreamWriter.awrite() first tries to write to a socket,
        # and if that succeeds, yield IOWrite may never be called
        # for that socket, and it will never be added to poller. So,
        # ignore such error.
        self.poller.unregister(sock, False)

The self.poller.unregister takes 2 arguments: sock & False.

I think the function definition that implements that method lives in extmod/moduselect.c (file), and it reads:

STATIC mp_obj_t poll_unregister(mp_obj_t self_in, mp_obj_t obj_in)

If I remove the False from the python function call, the crash is gone.

Will keep looking

ernstnaezer avatar Dec 26 '18 21:12 ernstnaezer

I can confirm the issue and also that it's gone when applying @ernstnaezer's fix

larsrinn avatar Dec 28 '18 17:12 larsrinn

it seems to go wrong on the last line of remove_writer:

It doesn't go wrong, it goes right. That's a recent fix applied for uasyncio to work under different circumstances, and efficiently in all cases: https://github.com/pfalcon/micropython-lib/commit/4f70217e4fce6a6ca3c50ea8552a8ec094af0d34 . (Efficiency is the primary goal of uasyncio, there're no compromises here.)

Once again, that requires up to date Pycopy, from https://github.com/pfalcon/pycopy , as specified in the README. Corresponding patch was submitted upstream long ago too: https://github.com/micropython/micropython/pull/4217

pfalcon avatar Dec 28 '18 18:12 pfalcon

I just built micropython from git using the AUR and this error still seems to be there? Using the non-git version I don't have access to ussl, so I don't have upip.

I can chalk a certain amount up to just archlinux weirdness, I suppose that's just a problem with source packages sometimes. But still, it's odd.

traverseda avatar Feb 15 '19 16:02 traverseda

People writing, or intended to write, here should instead go to https://github.com/micropython/micropython/pull/4217 and let the maintainers of that repo know that that unmerged patch causes them unneeded complications.

pfalcon avatar Feb 22 '19 18:02 pfalcon

Hi, I’ll try to write here because I think the problem that I'm having using Picoweb is similar to that described in this thread.

My test is done on the ESP32 module and I’m using the uPithon ver. 1.11 with the Picoweb and Uasyncio Ulogging, taken from pfalcon/Picoweb and pfalcon/pcopy-lib. As soon as I lunch the webserver example I have this output:

exec(open('./picoTest.py').read(),globals()) [0;32mI (81912) modsocket: Initializing [0m Running on picoweb/, in run_fo "/lib/uasyncio/init.py", line 31, in add_reader TypeError: function expected at most 3 arguments, got 4

At line 31 of uasyncio we have: self.poller.register(sock, select.POLLIN, cb)

I’d like to try to build the Pcopy executable but I think that it is a bit over my head; thank you in advance for any suggestion.

fgiava avatar Aug 28 '19 09:08 fgiava

The suggestion is to read README: https://github.com/pfalcon/picoweb/blob/master/README.rst, which states clearly that yes, Pycopy (https://github.com/pfalcon/pycopy) is required for Picoweb, as Picoweb is part of the Pycopy project (https://github.com/topics/pycopy).

pfalcon avatar Aug 28 '19 09:08 pfalcon

Hi, thank you for the prompt replay, summarizing, at this time there is not a workaround to run Picoweb with uPython (some month ago this option was possible) the suggested way is to build the Pcopy binary.

fgiava avatar Aug 28 '19 15:08 fgiava

As the author of both Pycopy and Picoweb, I intend Picoweb to be run with Pycopy, and I don't have resources to support other implementations. (Except for CPython, it's on my TODO to make Picoweb run there, by implementing set of Pycopy-compatible APIs for CPython.)

pfalcon avatar Aug 28 '19 15:08 pfalcon

Ok all clear, thank you @pfalcon .

Anybody here can share resources to make picoweb WORKING again for ESP32 boards?

Thanks to all, m

mauroesposito avatar Sep 03 '19 10:09 mauroesposito

@mauroesposito Picoweb is working on ESP32 boards. You just have to swap the base uPython. Picoweb is working with @pfalcon s uPython implementation named "Pycopy" it is based on Micropython as he was a long contributer to the project... (whole story in Pycopy FAQ). So, if you need it, change from Micropython to Pycopy that's it.

mrwhy-orig avatar Sep 06 '19 12:09 mrwhy-orig

Thank you @mrwhy-orig , in this moment I've no time to setup all for build: and until now, I believed that somewhere was possible found a *.bin ready to flash on the chip, nothing else.

mauroesposito avatar Sep 06 '19 14:09 mauroesposito

@pfalcon I can imagine you are sad or angry that micropython didn't take over your suggestion according asyncio. But to me and probably others, it's too great a risk and too much work to switch to an alternative for the micropython framework with a much smaller userbase, as opposed to the widely used and supported "official" micropython.

I took the chance, and it seems at least on today, I can use picoweb's great features just as easily with the slightly less perfect but de facto standard uasyncio (upip.install('uasyncio')).

Your picoweb is a wonderful and very elegant solution for those wanting to serve web pages (like me). I think it would greatly expand the user base of your software if you let people have the choice of which asyncio variant people use (your more perfected version, or the default micropython one) instead of pushing them into the pycopy direction. You could even point it out clearly in the documentation that you greatly recommend your implementation, and why.

zeekoe avatar Jun 20 '20 12:06 zeekoe

@pfalcon I can imagine you are sad or angry that micropython didn't take over your suggestion according asyncio. But to me and probably others, it's too great a risk and too much work to switch to an alternative for the micropython framework with a much smaller userbase, as opposed to the widely used and supported "official" micropython.

I took the chance, and it seems at least on today, I can use picoweb's great features just as easily with the slightly less perfect but de facto standard uasyncio (upip.install('uasyncio')).

Your picoweb is a wonderful and very elegant solution for those wanting to serve web pages (like me). I think it would greatly expand the user base of your software if you let people have the choice of which asyncio variant people use (your more perfected version, or the default micropython one) instead of pushing them into the pycopy direction. You could even point it out clearly in the documentation that you greatly recommend your implementation, and why.

@zeekoe: Let me be short and simple: if you would like to hire me to perform some work, certainly feel free to send your offer. If you not, feel free to use my works, in accordance with their licensing terms, which in particular require recognizing and upholding my copyright and authorship. In that regard, I've written the original uasyncio and picoweb, and I continue to maintain them, according to my approach and ability.

I'm sorry to hear that other projects cause your confusion. When I decided to go with my own MicroPython fork, the project initially was called "micropython-pfalcon", but I got a suggestion to rename it, and with that being a reasonable suggestion, I followed it and renamed the project to "Pycopy". I now return this suggestion to other projects which try to use names of my work(s) for their benefit, don't follow the approach of my original projects, don't give proper credit to me, and cause overall confusion in the community - please get some respect and use different names for derivative (or even original) work. For all concerned parties, I would also encourage them to look up definition of https://en.wikipedia.org/wiki/Plagiarism .

With this explanation, please don't come to me with terms like

de facto standard uasyncio (upip.install('uasyncio')).

For that sounds insulting, given that I'm the author of things mentioned there (including upip!), and by any account would be let to decide what they should be/how they should work, with other parties to pay at least the baseline respect for my work.

pfalcon avatar Jun 20 '20 13:06 pfalcon

Hi pfalcon, I'm sorry if I insulted you - I did not mean to do that in any way. I'm not in the position to hire you, just an enthousiastic hobbyist very happy that a form of python is available on the ESP32 platform. Thank you for the short history! Things are more clear to me now. The least I can do then is to express my thanks for your work; upip, too, is a wonderful tool.

zeekoe avatar Jun 20 '20 18:06 zeekoe