Skip to content

Comments

Add piano-keyboard config#46

Merged
marceloams merged 2 commits intoUselessCo:developfrom
WinnyChang:add-config-piano-keyboard
Oct 21, 2025
Merged

Add piano-keyboard config#46
marceloams merged 2 commits intoUselessCo:developfrom
WinnyChang:add-config-piano-keyboard

Conversation

@WinnyChang
Copy link
Contributor

Pull Request

Type of Change

  • Bug fix
  • New feature
  • Sound contribution
  • Configuration contribution
  • Documentation update
  • Other (specify)

Description

Add a new piano-keyboard.yaml config that uses the piano-keys sound files. Piano keys C3-E5 are mapped to the keyboard layout.

Related Issue

Related to #3.

Changes Made

  • Added a music-themed config file, piano-keyboard.yaml in src/configs/.

For Sound Contributions

  • Sound is in MP3 format
  • File size is under 500KB
  • Added sound to assets/sounds/
  • Created/updated config that uses the sound

Key bindings

Key bindings are illustrated in the image below:

piano-keyboard-key-binding

Checklist

  • My code/contribution follows the project guidelines
  • I have tested my changes
  • I have updated documentation as needed
  • My changes don't break existing functionality

@WinnyChang
Copy link
Contributor Author

@marceloams I noticed an issue with the maxConcurrent audio configuration. Even though I set maxConcurrent: 20 in the piano-keyboard config file, I still receive the warning "WARN Max concurrent sounds reached" when playing more than five sounds simultaneously. Is there an upper limit to the maxConcurrent value?

@marceloams
Copy link
Contributor

@WinnyChang Thanks for reporting this! The issue has been fixed, the config value was being overridden by a hardcoded default. Your maxConcurrent: 20 should work correctly now. Let me know if you encounter any other problems!

marceloams
marceloams previously approved these changes Oct 21, 2025
@WinnyChang WinnyChang dismissed marceloams’s stale review October 21, 2025 04:25

The merge-base changed after approval.

@marceloams marceloams merged commit 950e34b into UselessCo:develop Oct 21, 2025
11 checks passed
@marceloams
Copy link
Contributor

@WinnyChang Thanks for your contribution! Really nice piano setup! 🎹 🚀

@WinnyChang
Copy link
Contributor Author

@marceloams Thanks for the kind words! I really enjoyed contributing to this project. 🥳

Regarding the maxConcurrent issue, the warning still appears on my end. Here’s what I’ve done so far:

  1. Synced my fork and confirmed my local version is up to date.
  2. Installed again using npm install -g soundbind.
  3. Run Soundbind with soundbind src/configs/piano-keyboard.yaml.

Did I miss any steps?

Here's a screen recording showing the issue (the warning appears when I press the sixth key):

WARN-max-concurrent-sounds-reached.mov

@marceloams
Copy link
Contributor

Hey @WinnyChang! You're running the published npm package (soundbind command), which doesn't have the latest changes yet.

To test, use your local fork. Run from source instead:

npm run piano-keyboard

The soundbind command uses the published package. We'll update the package with all new contributions and fixed issues at the end of October!

@WinnyChang
Copy link
Contributor Author

@marceloams Thanks for clarifying the difference! I thought all the commands worked interchangeably. 😂

npm run piano-keyboard didn’t work, but I figured out that npm start piano-keyboard works perfectly! Looking forward to the package update at the end of October. 🤩

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.

2 participants