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

API updates #7

Closed
wants to merge 46 commits into from
Closed

API updates #7

wants to merge 46 commits into from

Conversation

jcharr1
Copy link
Collaborator

@jcharr1 jcharr1 commented Oct 25, 2021

-api update via @dfuchslin
-updated node-roon-api dependency to fix this.moo.close is not a function error on closing Roon.
-added Dockerfile

Fixes #6.

@docbobo
Copy link
Owner

docbobo commented Jan 17, 2025

@jcharr1 do you still want to get this merged into this extension? I don't have any Denon/Marantz device anymore to actually test anything? Which is also the reason why it took me... let's check... more than three years to respond...

@jcharr1
Copy link
Collaborator Author

jcharr1 commented Jan 17, 2025

@docbobo Since you don't have any devices to test yourself on anymore, I feel a little odd asking you merge this into your own repo now. I'll leave it up to you, though. I'm fine with either way.

@docbobo
Copy link
Owner

docbobo commented Jan 17, 2025

Fair enough. But I had some issues reported recently, and I don't want to leave everyone hanging any longer 😉 Plus, there also seem to be security issues.

What about this: I'll try to merge this into some separate branch first, spend some minor time on it, than push a build to docker that you can verify. Maybe we can move this into the roon extension manager repo afterwards.

@jcharr1
Copy link
Collaborator Author

jcharr1 commented Jan 17, 2025

Sounds good. 👍🏻

@docbobo
Copy link
Owner

docbobo commented Jan 17, 2025

Okay, can you check if docker.io/docbobo/roon-extension-denon:latest works for you? I've mostly cherry-picked all the commits that are not related to building the docker image; just the code changes.

The docker image itself is based on node:23-alpine3.20 instead of node:23. This saves a few hundred MB in image size.

If you are interested the code changes are in docbobo/bring-this-to-2025. I'll probably create a separate PR for that, tagging you there.

@jcharr1
Copy link
Collaborator Author

jcharr1 commented Jan 17, 2025

Seems to be working just fine on my Synology NAS. 👍🏻

@docbobo
Copy link
Owner

docbobo commented Jan 17, 2025

Awesome. That makes it already two, I guess. I'll probably go on and merge #10 then - I was mostly looking for some more general breakages.

@docbobo
Copy link
Owner

docbobo commented Jan 17, 2025

A few questions:

  • Do you mind checking if ghcr.io/docbobo/roon-extension-denon:latest also works for you?
  • Can I add you as collaborator on this repository?

@jcharr1
Copy link
Collaborator Author

jcharr1 commented Jan 17, 2025

  1. That one seems to be working fine for me, too.
  2. Sure. 😀

@docbobo
Copy link
Owner

docbobo commented Jan 17, 2025

Awesome on both ends. Just added you, you should've an invite. If something has to change, let's try to do that via a PR anyway, I would suggest. Unless it's something super urgent - in that case cowboy mode is also fine.

@docbobo
Copy link
Owner

docbobo commented Jan 17, 2025

Will close this PR now. Email is in the logs 😉

@docbobo docbobo closed this Jan 17, 2025
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.

don't work on recent Denon Receiver
4 participants