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

Python async support #79

Open
fidoriel opened this issue Dec 28, 2024 · 8 comments
Open

Python async support #79

fidoriel opened this issue Dec 28, 2024 · 8 comments
Labels
enhancement New feature or request

Comments

@fidoriel
Copy link

fidoriel commented Dec 28, 2024

Hey,

very useful library.

I am very keen on using this in an async context.
aiomqtt is a async python paho mqtt client, which could be used as a base.

As a first step I think pulling out the command creation logic out of PrinterMQTTClient and make a common logic base class out of it. Then async and sync child classes could be created. This would not break any current behavior/naming.

ftp and stream similar.
For my current project, I need an async impl. Forking, or copying commands seems not very elegant to me, so I created this pr. I am also willing to contribute if you are willing to merge ;)

@ohmdelta
Copy link
Collaborator

ohmdelta commented Jan 2, 2025

Async Client sounds good @mchrisgm

@mchrisgm
Copy link
Owner

mchrisgm commented Jan 4, 2025

Sounds interesting. Could be useful to add async support, but there will definitely be some timing concerns

@mchrisgm mchrisgm added the enhancement New feature or request label Jan 4, 2025
@fidoriel
Copy link
Author

fidoriel commented Jan 5, 2025

I implemented some commands manually and did not had any issues

@fidoriel
Copy link
Author

fidoriel commented Jan 5, 2025

How do we proceed?

I would do it like this

class PrinterCommandClass:
    def __generate_gcode_line(self, gcode_command: str) -> dict[str, Any]:
        return {"print": {"command": "gcode_line", "param": f"{gcode_command}"}}


class PrinterMQTTClient(PrinterCommandClass):
    def __send_gcode_line(self, gcode_command: str) -> bool:
        """
        Send a G-code line command to the printer

        Args:
            gcode_command (str): G-code command to send to the printer
        """
        return self.__publish_command(self.__generate_gcode_line(gcode_command))

class AsyncPrinterMQTTClient(PrinterCommandClass):
    async def __send_gcode_line(self, gcode_command: str) -> bool:
        """
        Send a G-code line command to the printer

        Args:
            gcode_command (str): G-code command to send to the printer
        """
        return await self.__publish_command(self.__generate_gcode_line(gcode_command))

I would also try to pull out all validation logic and state into the common class wherever possible.

@mchrisgm
Copy link
Owner

mchrisgm commented Jan 5, 2025

Will look into it

@fidoriel
Copy link
Author

Anything new?

@fidoriel
Copy link
Author

Would still like to contribute...

@ohmdelta
Copy link
Collaborator

If you need async context it's probably best that you create a separate async mqtt client.

Out of interest why do you need async context (if it's for webserver, we've had pretty much no issues with FastApi for example)? The general design of the Api has been to effectively try to recover the current state of the printer and then constantly update the state - when you query for the state you probably want the last recorded state rather than the next state. The place where async would perhaps be useful is verifying commands are published correctly ? If the problem is the wait time we can disable the wait_for_publish, after publishing messages.
aiomqtt from what I can see is effectively a wrapper for paho mqtt which is what we're using.

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

No branches or pull requests

3 participants