JoelBender / bacpypes

BACpypes provides a BACnet application layer and network layer written in Python for daemons, scripting, and graphical interfaces.
MIT License
294 stars 130 forks source link

Issue with first use of set_timeout of IOCB when updating from v0.16 to v0.18 #529

Open yujia21 opened 1 month ago

yujia21 commented 1 month ago

Hello, I'm updating dependencies for an application that receives requests from clients and then sends it on to a bacnet server using bacpypes. The "send" in question looks like this:

        iocb = IOCB(bacnet_request)
        iocb.set_timeout(TIMEOUT)
        deferred(_app.request_io, iocb)
        iocb.wait()
        if iocb.ioResponse or iocb.ioError:
            return iocb

Prior to the dependency update we were at v0.16.1 and didn't face any issues. On updating to v0.17 onwards, the first "send" always fails with

raise RuntimeError("no task manager")

but subsequent sends work well.

I'm not familiar enough with bacpypes to know if it's linked to a change between the versions and what to change on our side, any ideas?

JoelBender commented 1 month ago

The TaskManager is a singleton class that is created in run() if one hasn't already been created. The task manager is responsible for scheduling tasks like that timeout, and one hasn't been created yet.

The solution is to create one, e.g., from bacpypes.task import TaskManager; TaskManager(), in your main() before creating the IOCB and setting the timeout, or putting the code in some function like init_things_to_do() and calling deferred(...) to "schedule" the function call inside run(), which will be after the task manager is created.

Why this weird dance? I wanted the ability for developers to create a subclass of TaskManager with their own behavior and I use it for the tests, see the TimeMachine. This design is fine for what it is when it was written, but it also makes the library not thread safe.

Tasks and IOCBs all go away in BACpypes3 in favor of asyncio and you can do things like some_value = await app.read_property(...).

yujia21 commented 1 month ago

Much thanks. Wasn't aware that bacpypes3 existed, will definitely be checking it out.