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

Implement Button+ V1.12 API and introduce compatibility layer #66

Merged
merged 29 commits into from
Aug 1, 2024

Conversation

scspijker
Copy link
Contributor

@scspijker scspijker commented Jul 24, 2024

I've started to implement the V1.12 API and introducing a compatibility layer so it's easier to implement new versions.

For now this is a draft PR, so other maintainers can track progress and send early feedback.

So far I've:

  • Introduced Firmware version based model detection
  • Introduced unit tests for parsing JSON responses
  • Introduced a Docker Compose script for easy testing of the integration
  • Introduced V1.12 JSON parsing
  • Implemented the new interface model in the domain layer 🔨

Thanks, and let me know if something could be better! This is my first Python code / contribution ever 💪

@koenhendriks koenhendriks self-requested a review July 25, 2024 06:47
@koenhendriks koenhendriks added enhancement New feature or request development Related to development of the integration labels Jul 25, 2024
@koenhendriks koenhendriks added this to the Version 1.0.0 milestone Jul 25, 2024
@scspijker scspijker marked this pull request as ready for review August 1, 2024 15:19
@scspijker
Copy link
Contributor Author

scspijker commented Aug 1, 2024

So, it's completely working now on v1.12.2. But Firmware v1.13 came out and broke shizzle again 😂
Firmware v1.12.3 seems to reboot the device when the config is pushed 😬

@koenhendriks
Copy link
Owner

I explicitly asked for changes between 1.12 and 1.13 before the release and to quote the creator of Button+:

No changes in the config between 1.12 and 1.13

Do you think you can add the 1.13 as well?

koenhendriks
koenhendriks previously approved these changes Aug 1, 2024
Copy link
Owner

@koenhendriks koenhendriks left a comment

Choose a reason for hiding this comment

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

Thanks a lot for your work on this! I hope this will help fix config changes faster in the future.

Really nice that you also added tests for this 👍

@koenhendriks
Copy link
Owner

@scspijker Just 1 failing CI pipeline I see. Could you take a look? I'm not really sure what the CI actually tries because it was added by @bolkedebruin

@scspijker
Copy link
Contributor Author

@scspijker Just 1 failing CI pipeline I see. Could you take a look? I'm not really sure what the CI actually tries because it was added by @bolkedebruin

I'll take a look 👍

@scspijker
Copy link
Contributor Author

@koenhendriks CI should be fixed, and now also runs the pytest automatically 💪

Let's fix 1.13 in a new PR.

auto-merge was automatically disabled August 1, 2024 19:55

Head branch was pushed to by a user without write access

@scspijker
Copy link
Contributor Author

@koenhendriks okay, let's try that one more time (hopefully 😂 ). After doing some setup I could run the GHA's in my own repo, and the build now works there. The latest ubuntu runner image uses Python 3.10, but a minimum of 3.11 is required for Home Assistant. This is now setup in build.yaml, also caching dependencies.

@koenhendriks
Copy link
Owner

I might take a look at the settings of the repo, not sure why I should manually give permission for each CI run...

@scspijker
Copy link
Contributor Author

scspijker commented Aug 1, 2024

I might take a look at the settings of the repo, not sure why I should manually give permission for each CI run...

You have limited "free" minutes with open source repo's. So somebody with permissions (like CODE-OWNER) has to approve before CI runs. Else bad actors can use all your free minutes by just opening 200 PR's at once.

CI checks passed 🎉

@koenhendriks koenhendriks merged commit 0a1e2d4 into koenhendriks:main Aug 1, 2024
4 checks passed
@IBlackI
Copy link

IBlackI commented Aug 2, 2024

Is it the plan to create a (beta) release with these changes?

@koenhendriks
Copy link
Owner

Just released v0.0.15

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

Successfully merging this pull request may close these issues.

3 participants