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

ensure 2 digits for volumeHex #71

Merged
merged 2 commits into from
Nov 25, 2020
Merged

Conversation

wweich
Copy link
Contributor

@wweich wweich commented Nov 25, 2020

Status

READY

Migrations

NO

Description

For volume level below 16, the MVL command currently only has 1 digit. This doesn't work (for my ONKYO receivers). It has to be 2 digits.

Steps to Test or Reproduce

Try setting the volume below 16 before and after the fix.

@wweich wweich requested a review from jupe as a code owner November 25, 2020 10:21
@jupe
Copy link
Owner

jupe commented Nov 25, 2020

TypeError: volume.toString(...).toUpperCase(...).padStart is not a function

CI seems to use node.js 6 that doesn't support padStart. node.js v8 support so we should update it, which would be good anyway.

@jupe
Copy link
Owner

jupe commented Nov 25, 2020

update node.js here: #72

@coveralls
Copy link

Pull Request Test Coverage Report for Build 241

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 46.789%

Totals Coverage Status
Change from base Build 240: 0.0%
Covered Lines: 221
Relevant Lines: 444

💛 - Coveralls

@jupe jupe merged commit 6994ddb into jupe:master Nov 25, 2020
@wweich wweich deleted the fix-setVolume-below-16 branch November 25, 2020 12:21
wweich referenced this pull request in wweich/onkyo.js Apr 16, 2022
jupe pushed a commit that referenced this pull request Sep 8, 2022
* ensure 2 digits for volumeHex (#71) (again)

* fix return for setVolume (#73) (again)

* fix test
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.

3 participants