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

Switch to v1 API endpoints and use DigestAuth #63

Merged
merged 11 commits into from
Nov 27, 2023

Conversation

Skaronator
Copy link
Contributor

I was able to get DigestAuth working with aiohttp. Additionally, I've switched to v1 endpoints since they give us much more information about the status of the printer. Most importantly for me is the new Printer State which shows us ATTENTION and READY that are currently missing in the old endpoint and therefor also in Home Assistant.

I've tested these changes on my Prusa XL with Firmware 4.7.2. The new /v1/ endpoints should be available for MK4, Mini and XL so nothing change there in terms of compatibility with this library.

Prusa documentation is really confusion, but it seems like that the old non v1 API Endpoints will be removed at some point. The help page already said that the old UI (that uses the old endpoints) is already gone: https://help.prusa3d.com/article/prusa-connect-local-mini_133279
And the Firmware Code already mentions that the printer state information from the old API will get removed: https://github.com/prusa3d/Prusa-Firmware-Buddy/blob/bfb0ffc745ee6546e7efdba618d0e7c0f4c909cd/lib/WUI/link_content/basic_gets.cpp#L120-L127

You can test these changes locally by creating a main.py in the root directory with this content:

#!/bin/env python3
from pyprusalink import PrusaLink
import aiohttp
import asyncio


async def main():
  async with aiohttp.ClientSession() as session:
    api = PrusaLink(session,"http://prusaxl.fritz.box", "username", "password")
    version = await api.get_version()
    print("version: ", version, "\n")

    info = await api.get_info()
    print("info: ", info, "\n")

    status = await api.get_status()
    print("status: ", status, "\n")

    job = await api.get_job()
    print("job: ", job, "\n")

    if 'file' in job:
      thumb = await api.get_file(job['file']['refs']['icon'])
      print(thumb)

    if 'file' in job:
      thumb = await api.get_file(job['file']['refs']['thumbnail'])
      print(thumb)


if __name__ == "__main__":
    import asyncio
    asyncio.run(main())

When this gets merged I'd suggest bumping the version to 2.0.0, and I'd like to do a PR in Home Assistant to accommodate these changes.

@Skaronator
Copy link
Contributor Author

Skaronator commented Oct 30, 2023

I'm currently trying to implement this in home assistant and I don't understand why, but the session logic stopped working when this PR is used within home assistant code base. Every second API call will get a 401 response.

@Skaronator Skaronator marked this pull request as draft October 30, 2023 22:09
@Skaronator
Copy link
Contributor Author

It looks like it works now. Also got the home assistant changes ready but going to test them in the following days.

@Skaronator Skaronator marked this pull request as ready for review October 31, 2023 17:44
@Skaronator
Copy link
Contributor Author

Hi @balloob, do you mind taking a look over it? (and maybe also the home-assistant/core#103396 PR)

@rforro
Copy link

rforro commented Nov 7, 2023

I'm being curious which api version number is the reporting your XL when requesting /api/version ?

@rforro
Copy link

rforro commented Nov 7, 2023

I see, that you removed old non-V1 endpoints. Maybe is a good idea to keep then until they will be removed for good by Prusa. I'm not sure if all printers were switched to V1.

Look at Prusa-Link-Web frontend they enabled V1 for Buddy FW MINI, MK3.5, MK4 https://github.com/prusa3d/Prusa-Link-Web/blob/377e1509542100efe1689415d603d7a330d28d3c/config.mini.js#L25 but not yeat for SL1 https://github.com/prusa3d/Prusa-Link-Web/blob/master/config.sl1.js

@Skaronator
Copy link
Contributor Author

I'm being curious which api version number is the reporting your XL when requesting /api/version ?

{
	"api": "2.0.0",
	"server": "2.1.2",
	"nozzle_diameter": 0.40,
	"text": "PrusaLink",
	"hostname": "PrusaXL",
	"capabilities": {
		"upload-by-put": true
	}
}

The API features between Prusa Mini, MK3.9, MK4 and XL should be identically. The firmware was originally written for the Mini then they added support for MK4 and later for XL in the same codebase. It's been a while since they released a new firmware for the mini, but once the 5.1 is out of alpha they all share the same version.

The SL1 is currently not supported in the main branch of this library either, since they require HTTP digest auth. So nothing changes there in compatibility.

@rforro
Copy link

rforro commented Nov 7, 2023

I'm being curious which api version number is the reporting your XL when requesting /api/version ?

{
	"api": "2.0.0",
	"server": "2.1.2",
	"nozzle_diameter": 0.40,
	"text": "PrusaLink",
	"hostname": "PrusaXL",
	"capabilities": {
		"upload-by-put": true
	}
}

The API features between Prusa Mini, MK3.9, MK4 and XL should be identically. The firmware was originally written for the Mini then they added support for MK4 and later for XL in the same codebase. It's been a while since they released a new firmware for the mini, but once the 5.1 is out of alpha they all share the same version.

The SL1 is currently not supported in the main branch of this library either, since they require HTTP digest auth. So nothing changes there in compatibility.

Yes, as I wrote on the other PR V1 or non-V1, it will report 2.0.0 everytime.

But when your PR will be merged the HTTP digest auth will be supported, so by keeping non-V1 endpoint, you'll solved SL1 support as well.

@Skaronator
Copy link
Contributor Author

Leaving the non-V1 endpoint doesn't really solve SL1 support, since the home assistant integration uses the new endpoints exclusively, and I cannot test the SL1 support since I don't own one. But @edenhaus suggested in the other PR that we keep the old API endpoint for the material type. I might bring back some old endpoints, but it's getting confusing with the names of the Types. 😅

@rforro
Copy link

rforro commented Nov 7, 2023

Yes it could get bit complicated as I think that not everything is currently implemented in V1. For example I cannot find ambient temperature in V1 endpoints.

I would create types_legacy.py for non-V1 and types.py for V1. After authenticating it could test if V1 exists and use them instead. If not, it will fallback to non-V1. This will solve also situations like when V1 is worse then non-V1.

@Skaronator
Copy link
Contributor Author

I have added back an endpoint used for the material type sensor using the old API endpoint. I think that's enough for this and the Home Assistant PR for now.

If someone wants to implement logic for the SL1 they can do that once this PR was merged. It's already a step closer to the SL1 thanks to the Digest Auth implementation.

Copy link
Contributor

@agners agners left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks quite good to me. I've tried your example against my MK3.9, working fine 🎉

pyprusalink/types.py Outdated Show resolved Hide resolved
pyprusalink/client.py Outdated Show resolved Hide resolved
@rforro
Copy link

rforro commented Nov 22, 2023

Do you completely removed login by API key? Because currently the latest MINI firmware 4.4.1 is using API only.

@agners
Copy link
Contributor

agners commented Nov 22, 2023

Do you completely removed login by API key? Because currently the latest MINI firmware 4.4.1 is using API only.

Hm, I see, from what I can tell that firmware did not support the digest auth method yet. I guess we can make it support both methods for now 🤔

@Skaronator
Copy link
Contributor Author

Do you completely removed login by API key? Because currently the latest MINI firmware 4.4.1 is using API only.

Didn't know that the Mini doesn't have Digest Auth yet. Personally, I'd just wait until the next firmware is stable. Then all printers (XL, MK4 and Mini) have the same codebase. The firmware is already in beta: https://github.com/prusa3d/Prusa-Firmware-Buddy/releases

The new Prusa Slicer 2.7 enables binary gcode by default, therefor making it somewhat mandatory for all users to update the firmware anyway. Additionally, the new firmware provides input shaper for the mini, making it a no-brainer update.

@agners
Copy link
Contributor

agners commented Nov 22, 2023

At least at MK3S+ times, firmware updates weren't really forced by Prusa, so I am quite sure that there are many MK3S+ with older firmwares. Now I know that this doesn't affect PrusaLink, as MK3S+ don't have native PrusaLink support. But extrapolated from that, I'd say it is quite possible that users have Mini's with an old firmware for quite some time still, possibly also using older Prusa Slicers etc.

I am not generally against removing old code/break support for older models/firmware. However, I feel supporting API authentication is relatively simple thing to do, and would unblock merging this PR already today... We can also test this with today's firmware easily. What do you think @Skaronator

@Skaronator
Copy link
Contributor Author

I'm not even sure if the current mini firmware provides all v1 endpoints, when the current firmware doesn't even support DigestAuth.
We could do some logic to use legacy endpoints when v1 doesn't exist. But since non-v1 endpoints are deprecated and the fact that I don't own a Mini to even test this I don't really want to do that.

@agners
Copy link
Contributor

agners commented Nov 22, 2023

I'm not even sure if the current mini firmware provides all v1 endpoints, when the current firmware doesn't even support DigestAuth.

Yeah, judging from https://github.com/prusa3d/Prusa-Firmware-Buddy/blob/v4.4.1/lib/WUI/link_content/prusa_link_api.cpp#L40 it really looks like v1 is not available 😢

We could do some logic to use legacy endpoints when v1 doesn't exist. But since non-v1 endpoints are deprecated and the fact that I don't own a Mini to even test this I don't really want to do that.

I definitely would prefer that. I do run the PrusaLink support with my MK4, so it seems that the legacy endpoints are supported on the newer firmware. So we can test it against that.

From this YT video it seems that the Mini input shaper is also not in best shape (no pun intended 😅 ). I am worried that it might take a while until Mini firmware gets stable, and I prefer to not be dependent on other people's timeline. Also, breaking changes are really not that popular, so if possible I'd prefer to prevent it.

I think for this library it should be quite straight forward to make it backwards compatible (in worst case we just separate the code into separate classes. Then we can see how things go on Core side.

pyprusalink/client.py Outdated Show resolved Hide resolved
@Skaronator
Copy link
Contributor Author

@agners Firmware 5.1 is now stable for XL, MK4 and Mini making v1 endpoints available on all printers.

@Skaronator Skaronator requested a review from agners November 24, 2023 14:59
pyprusalink/client.py Outdated Show resolved Hide resolved
pyprusalink/client.py Outdated Show resolved Hide resolved
@Skaronator
Copy link
Contributor Author

Code formatting should be fixed now. A bit annoying that CI doesn't run automatically until I had contributed (merged) anything.

@agners
Copy link
Contributor

agners commented Nov 24, 2023

I gave it a stab too support both APIs see #67. A simple test script seems to work against this version as well as the current released version, so this can be a drop in replacement.

From that version we then can decided what we do on Core level. I still would prefer if we would also support the old firmware. According to analytics we have more than 500 users using PrusaLink integration, presumably quite some with older firmwares.

@Skaronator this is all based on your work, if it is fine by you, I'd just push that last commit on your branch and then merge your PR. Thoughts?

@Skaronator
Copy link
Contributor Author

I'd leave it as it is. Supporting non-v1 endpoints would be a lot of temporary code in the home-assistant core and I, personally, don't want to do any more work for now.

This breaking change would affect HASS 2024.1 or later and I argue that plenty of people update their firmware and slicer over the Christmas holidays.

Nonetheless, I've asked Prusa if they can estimate how fast people update their firmware/slicer.

@balloob
Copy link
Contributor

balloob commented Nov 27, 2023

If the new endpoints are available as an update to all printers, it's okay to no longer support it. However, Home Assistant should raise a repair issue for users that notifies them that they require a firmware update and link them to the Prusa help article on how to upgrade.

Copy link
Contributor

@agners agners left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. In this case let's merge this as is and release a 2.0.0 🎉

@balloob balloob merged commit 3675a1c into home-assistant-libs:main Nov 27, 2023
2 checks passed
@Skaronator Skaronator deleted the v1-api branch November 27, 2023 19:45
@balloob
Copy link
Contributor

balloob commented Nov 27, 2023

@Skaronator
Copy link
Contributor Author

Awesome! I'll update the home assistant core PR :)

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

Successfully merging this pull request may close these issues.

4 participants