Added basic integration to the Spoolman filament manager

[Donkie]

https://github.com/Donkie/Spoolman

The component pretty much does three things:

1. Act as a proxy for Spoolman's REST API, so that frontends doesn't need to be configured or worry about authentication to the Spoolman server. Frontends can e.g. query "/spoolman/spool" to get a list of spools.
2. Log spool usage similar to how the old integration attempts do it.
3. Handle active spool ID. Set by the /spoolman/set_spool endpoint.

Regarding 3., I've not yet settled on whether this is something Moonraker should keep track of, or whether Spoolman should be extended to also have a database of printers and spools assigned to them. It would make the interaction with Spoolman more complicated but perhaps the community is looking for a more complete system anyway?

Spoolman itself is also very WIP, but my aim is to start off small and see what needs there are, both from users and from moonraker/frontend devs and develop from that.

Heavily based off of #123 and #597 so many thanks to them for the base code.

Example moonraker.conf:

[spoolman]
server: http://192.168.0.100:7912

[Donkie]

Been testing this for a couple of prints now and I can confirm that it is working as expected. The updated filament weight after a print in the Spoolman db corresponds pretty much exactly with the estimated weight calculated by my slicer.

[cstueckrath]

could you provide basic [spoolman] configuration examples for moonraker.conf, please?

[Donkie]

could you provide basic [spoolman] configuration examples for moonraker.conf, please?

Sure! I have added it to the issue description.

[cstueckrath]

thanks, so I guessed that correctly ;). I've noticed there is no easy way on spoolman's side to set the "mounted" spool, and as there are no ways from any UIs yet to do so, trying that from a workflow perspective seems to be quite hard. Do you use klipper macros to set the active spool? How is your workflow?

[Donkie]

thanks, so I guessed that correctly ;). I've noticed there is no easy way on spoolman's side to set the "mounted" spool, and as there are no ways from any UIs yet to do so, trying that from a workflow perspective seems to be quite hard. Do you use klipper macros to set the active spool? How is your workflow?

Yup, that is a known problem. As I mentioned in the description, I'm not settled yet on how this is supposed to work. Either If that is something that is supposed to be set from Spoolman's side, then Spoolman needs a database of printers and everything surrounding that, which makes it's scope increase drastically. Not necessarily a bad thing, but do people want a more complete platform for all the inventory, or would it make it too complicated is the question.

Alternatively, it's something that's handled in each printer's database, like Moonraker in this case. Which is how it currently works.

This is something I would like some input on from the community.

And yes, since it's so new, there is no UI for this.

My workflow currently is to set this manually using a POST request to <moonraker_ip>:<moonraker_port>/spoolman/set_spool, with the json payload: {"spool_id": 123}

[solo2424]

I have Spoolman installed and I added the [spoolman] to the moonraker.conf, is there any interface within mainsail? I don't see any additions. I went through the settings and I do not see anything.

[Donkie]

@solo2424 Please read the above comment. There is no UI yet since it's not integrated into any frontend yet.

Currently, the only way to set the spool is by sending manual POST requests.

[solo2424]

Thanks, sorry I missed that. So I run a post request with the instructions you gave, I am getting the following error. In advance, I do apologize for my ignorance. If you don't wish to waste your time helping a novice, I get it.

POST 192.168.0.93:7126/spoolman/set_spool
Please enter content (application/x-www-form-urlencoded) to be POSTed:
{"spool_id": 1}{"error": {"code": 404, "message": "Not Found", "traceback": "Traceback (most recent call last):\n\n File "/home/solo/moonraker-env/lib/python3.9/site-packages/tornado/web.py", line 1690, in _execute\n result = self.prepare()\n\n File "/home/solo/moonraker/moonraker/app.py", line 962, in prepare\n raise tornado.web.HTTPError(404)\n\ntornado.web.HTTPError: HTTP 404: Not Found\n"}}solo@SoLoServer:~$

[solo2424]

I beleive my issue may have to do with these errors. I looked a the log but I don't see anything that sticks out to me.

An error was detected while loading the moonraker component 'spoolman'. Please check the log file and fix the issue.

Unparsed config section [spoolman] detected. This may be the result of a component that failed to load. In the future this will result in a startup error.

moonraker.log

[Donkie]

@solo2424 the changes presented here are not yet live in Moonraker. You have to fetch my branch until @Arksine has reviewed and merged this.

I'm not sure how you have installed Moonraker, but if you have it in the form of a git repository, then you can follow this guide on how to switch Moonraker to use my branch instead: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally#modifying-an-inactive-pull-request-locally

[solo2424]

That did it! Thanks for your patiences and thank you for taking on this project. I think this has a lot of potential and the current state is well worth it. I am shocked that we haven't have more people take on filament management in klipper in the past. I know there was a project started awhile back that never picked up steam. @Arksine , I really hope you decide to implement this.

[Arksine]

Endpoints that use regular expressions won't work correctly with all transports. It is either necessary to limit the endpoint to the http transport or remove the end of string character.

[Donkie]

Ah ok, this regex won't be necessary when I remove the dynamic proxy path so this won't be an issue then.

[Arksine]

Moonraker doesn't support PUT and PATCH methods. A better solution for proxying a request might be to have the proxied request method and path as arguments to a single POST request, as most Moonraker applications rely on the websocket transport for API requests.

[Donkie]

Good idea, that makes it less janky

[Arksine]

I think it would be wise to set a minimum value here. Perhaps at 1 second?

[Donkie]

Good idea

[Arksine]

Since the server option is required we can leave out the default value and let the ConfigHelper raise an exception.

[Donkie]

Ah of course

[Arksine]

If spoolman only stores one key then we likely don't need to register a new namespace. We can use the moonraker namespace and store it in the spoolman.spool_id key.

Also we can avoid accessing the DB for each update and keep the spool ID cached in a class attribute (ie: self.spool_id), initialized in component_init(). For example, the following would work to initialize and update the spool id:

async def component_init(self):
    database: MoonrakerDatabase = self.server.lookup_component("database")
    self.spool_id = await database.get_item("moonraker", "spoolman.spool_id", None)

def set_active_spool(self, spool_id: Optional[str]) -> bool:
    database: MoonrakerDatabase = self.server.lookup_component("database")
    def database.insert_item("moonraker", "spoolman.spool_id", spool_id)
    self.spool_id = spool_id
    self.server.send_event(
        "spool_manager:active_spool_set", {"spool_id": spool_id}
    )
    logging.info(f"Setting active spool to: {spool_id}")
    return True

[Donkie]

Very good points

[Arksine]

Thanks. I did a quick review and posted some initial feedback above. If the goal is integration with a frontend then the module will likely also need to provide an API to query the current spool (can just add a GET to /spoolman/spool_id) and websocket notifications when the spool ID changes.

The spoolman module will also need to be documented (configuration, API, and changelog) before merging.

All that said, there are two things to consider as well:

1. I intend to add support for 3rd party plugins, however I don't have hard time on when I'll have it done. When that does occur I will likely move components that integrate with 3rd party software out and give ownership to the 3rd party, enabling them to make changes/updates without waiting on a pull request.
2. Have you considered having Spoolman connect directly to instances of Moonraker via websocket? This is what Obico does to integrate with Moonraker and gives them full control to all of the data they need. They register as an agent which provides a method to communicate with front ends.

Just to add, nothing about the above is intended to discourage merging this. I think this module is a useful addition to Moonraker and "Spoolman" is the best proposed way of doing spool tracking that can synchronize state with multiple printers. There are downsides to the agent strategy, specifically that the third party app needs to maintain connection state.

[Donkie]

@Arksine
Thank you for your thorough review. I believe I have now addressed all of your points.

I have not considered the agent approach. It does sound interesting and it would make things less coupled from a Moonraker perspective. But as you said, there is the inherent downside that if Spoolman loses connection in the middle of a print, important extruder events might be missed and the tracking is lost.

I think a good approach is to start with this type of integration, and then we can look into later to reverse it and become an agent if we see the need for it. That's something that can be phased in without breaking backwards compatibility.

The 3rd party plugins system sounds interesting. How do you envision that would work? How would plugins be connected?

[solo2424]

These are the commands I ran last time. I did the same and you can see the results. I tried running the updated curl and I get an error. But when I run the old command, It works. I have a feeling I didn't get your latest update.

solo@SoLoServer:~$ cd moonraker

solo@SoLoServer:~/moonraker$ git remote add fork https://github.com/Donkie/moonr
error: remote fork already exists.

solo@SoLoServer:~/moonraker$ git fetch fork spoolman
From https://github.com/Donkie/moonraker

• branch spoolman -> FETCH_HEAD

solo@SoLoServer:~/moonraker$ git checkout spoolman
Already on 'spoolman'
Your branch is up to date with 'fork/spoolman'.

solo@SoLoServer:/moonraker$ curl -X POST -H "Content-Type: application/json" -d '{"spool_id": 6}' http://192.168.0.93:7126/spoolman/spool_id
{"error": {"code": 404, "message": "Not Found", "traceback": "tornado.httpclient.HTTPClientError: HTTP 404: Not Found\n\n\nThe above exception was the direct cause of the following exception:\n\n\nTraceback (most recent call last):\n\n File "/home/solo/moonraker/moonraker/app.py", line 693, in _process_http_request\n result = await self._do_request(args, conn)\n\n File "/home/solo/moonraker/moonraker/app.py", line 669, in _do_local_request\n return await self.callback(\n\n File "/home/solo/moonraker/moonraker/components/spoolman.py", line 174, in _proxy_spoolman_request\n if query is not None:\n\n File "/home/solo/moonraker/moonraker/components/http_client.py", line 363, in raise_for_status\n raise ServerError(msg, code) from self._error\n\nmoonraker.utils.ServerError: Not Found\n\n\nThe above exception was the direct cause of the following exception:\n\n\nTraceback (most recent call last):\n\n File "/home/solo/moonraker-env/lib/python3.9/site-packages/tornado/web.py", line 1713, in _execute\n result = await result\n\n File "/home/solo/moonraker/moonraker/app.py", line 659, in post\n await self._process_http_request()\n\n File "/home/solo/moonraker/moonraker/app.py", line 695, in _process_http_request\n raise tornado.web.HTTPError(\n\ntornado.web.HTTPError: HTTP 404: Not Found\n"}}solo@SoLoServer:/moonraker$

solo@SoLoServer:~/moonraker$

solo@SoLoServer:/moonraker$ curl -X POST -H "Content-Type: application/json" -d '{"spool_id": 6}' http://192.168.0.93:7126/spoolman/set_spool
{"result": true}solo@SoLoServer:/moonraker$

[Arksine]

Use self.server.error instead of ServerError. Likewise it is probably not necessary to import ServerError for Type Checking since it isn't used in any annotations.

[Donkie]

Done!

[Arksine]

I don't think we should raise the exception here as it will spam the log if Spoolman is down. You could do something like the following:

if response.has_error():
    return

[Donkie]

I don't like fully repressing the error since then you can't debug it. I've made it now so it prints the first error since it was last successful, sounds good?

[Arksine]

The send_event method returns a future that can be awaited for control flow, however I don't think we need to be concerned with it here as spoolman doesn't depend on the outcome. It should be enough to fire and forget.

Additionally, I think that perhaps we we need to call track_filament_usage before setting the new spool if the previous spool_id is not None, and reset self.extruded to 0 otherwise. This way we flush any residual filament usage and avoid including extrusion moves that isn't part of the new spool.

[Donkie]

Good points

[Arksine]

Rather than return True I would recommend just returning the spool ID for both the GET and POST requests.

[Donkie]

Good point

[Arksine]

It may be better to name the argument request_method rather than method, so as not to confuse it with JSON-RPC's method parameter.

[Donkie]

Ah, good point

[Arksine]

It looks close to merging, I had a few comments above, mostly minor in nature.

With regard to plugins, my plan is for them to be very much like components, however they will be packaged and hosted on GitHub. Moonraker will use pip's VCS support to install and update them. I intend to extend moonlight to keep a curated list of extensions, any registered extension in this list can be installed/upgraded through the API. It will be possible to install extensions that aren't curated, however only via SSH.

[Donkie]

@Arksine I've addressed your review comments now, and also added configuration, web api and changelog documentation.

[Arksine]

Thanks. The PR looks good so I squashed and merged in commits 4b9c836 and e6b3b8b.

I also added a couple of small fixes in 5d54a10 that I noticed. It was so close I didn't want to bother with another review and delay further.

[Donkie]

Sick! Thank you :)

[Arksine]

FYI, I just pushed a commit that registers a remote method named spoolman_set_active_spool. This will allow Klipper to set the current spool ID using a gcode command, which I suspect will be ease in new users while they wait for integration from the frontends. This can also be added to macros such as UNLOAD_FILAMENT to clear the spool ID before performing the large negative extrusion.

Details on how to configure and use this have been added to Moonraker's docs.

[Donkie]

Oh nice, great idea!