[synth] adding Yamaha SY22, SY35, TG33, SY55, TG55, SY77, TG77, SY85, TG500 - #551
Open
hmmbug wants to merge 13 commits into
Open
[synth] adding Yamaha SY22, SY35, TG33, SY55, TG55, SY77, TG77, SY85, TG500#551hmmbug wants to merge 13 commits into
hmmbug wants to merge 13 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughAdds shared Yamaha SY/TG SysEx support and new model adaptations for voice detection, dump conversion, bank mapping, program changes, naming, module wiring, and test fixtures for several Yamaha synthesizers. ChangesYamaha SY/TG Synth Support
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Host
participant ModelAdapter
participant YamahaSYTGBase
participant YamahaSynth
Host->>ModelAdapter: request program or bank dump
ModelAdapter->>YamahaSYTGBase: build model-specific SysEx request
YamahaSYTGBase->>YamahaSynth: send request
YamahaSynth-->>YamahaSYTGBase: return voice or bank SysEx data
YamahaSYTGBase->>ModelAdapter: validate, split, and recalculate checksums
ModelAdapter-->>Host: expose patches, names, and program mappings
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@adaptations/Yamaha_SY77.py`:
- Around line 84-95: convertPatchesToBankDump is clobbering per-patch metadata
(device id, memory type and slot number) which breaks round-tripping with
extractPatchesFromBank; stop overwriting those bytes — do not assign
buf[OFFSET_DEVICE_ID], buf[self.offset_memory_type] or
buf[self.offset_memory_number] in convertPatchesToBankDump, instead preserve the
bytes already in each patch, then recompute and store the checksum with
OFFSET_CHECKSUM = self._calculateChecksum(buf) before appending to bank; keep
the rest of the loop and return behavior unchanged.
In `@adaptations/yamaha/Yamaha_SY_TG_common.py`:
- Around line 196-198: In channelIfValidDeviceResponse, when buf is too short
you currently return False which coerces to 0; change that to return -1 to
indicate "no match" and preserve the function's int return contract (update the
early-return in channelIfValidDeviceResponse to return -1 instead of False and
ensure any callers expect/handle -1 as the invalid/no-match sentinel).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f74b9b4e-6fa8-4368-b0dc-b1b59120090c
📒 Files selected for processing (7)
adaptations/Yamaha_SY77.pyadaptations/Yamaha_TG77.pyadaptations/testData/Yamaha_SY77/bank.syxadaptations/testData/Yamaha_TG77/Preset1.syxadaptations/testData/Yamaha_TG77/Preset1_01_SP_Cosmo__.syxadaptations/yamaha/Yamaha_SY_TG_common.pyadaptations/yamaha/__init__.py
hmmbug
marked this pull request as draft
June 6, 2026 22:16
hmmbug
changed the title
hmmbug
changed the title
hmmbug
changed the title
Author
|
The SY77 adaptation has been in use for a few weeks now and seems stable. Taking out of draft for review. |
hmmbug
marked this pull request as ready for review
July 11, 2026 14:07
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
adaptations/testData/Yamaha_SY22_SY35_TG33/ext.py (1)
38-38: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRename unused loop variable
xto_.♻️ Proposed fix
- for x in range(4): + for _ in range(4):🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@adaptations/testData/Yamaha_SY22_SY35_TG33/ext.py` at line 38, Rename the unused loop variable x to _ in the four-iteration loop, preserving the loop count and body behavior.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@adaptations/testData/Yamaha_SY22_SY35_TG33/ext.py`:
- Line 39: Update the patch construction in the surrounding extraction logic to
slice exactly patch_len bytes from bank starting at offset, removing the -1
adjustment. Preserve the existing prefix and suffix concatenation and offset
advancement so each voice slot yields all 574 bytes.
In `@adaptations/Yamaha_SY55.py`:
- Around line 108-119: Update friendlyProgramName to use the full SY55 slot
range by replacing the bankslot calculation based on memnum % 16 with memnum +
1. Preserve the existing memtype mapping, two-digit formatting, logging, and
returned name format so slots display I01–I64 or P01–P64 without wrapping.
In `@adaptations/Yamaha_SY77.py`:
- Around line 166-173: Update the patch-number validation around the visible
patchno mapping so negative values are rejected before the INTERNAL range maps
them to a tuple. Ensure invalid negative patchno values raise the existing
ValueError and cannot reach createCustomProgramChange() as MIDI data, while
preserving the current mappings for valid ranges.
In `@adaptations/Yamaha_SY85.py`:
- Around line 144-150: Update createProgramDumpRequest in the drum-voice branch
to replace the request identifier at the correct offset, changing the existing
“VC” bytes at indices 12–13 to “DR” without modifying the padding. Preserve the
current patch-number condition and return behavior.
In `@adaptations/Yamaha_TG33.py`:
- Around line 65-68: Rename the YamahaTG33 class attribute msg_id_alt_bulk_dump
to msg_id_alt_all_voice_dump while preserving the "PK 2203VM" value, so
YamahaSY22.extractPatchesFromBank() recognizes TG33 bank dumps.
In `@adaptations/yamaha/README_SY_TG_SYSEX.md`:
- Around line 6-29: Complete the NOTES column for every table row in the Yamaha
SY/TG SYSEX documentation by adding the missing trailing empty cell (`| |`)
where needed. Ensure the Voice Data Dump row also ends with a closing pipe after
its existing note, resolving the MD056 formatting issue without changing table
content.
---
Nitpick comments:
In `@adaptations/testData/Yamaha_SY22_SY35_TG33/ext.py`:
- Line 38: Rename the unused loop variable x to _ in the four-iteration loop,
preserving the loop count and body behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2c45e66d-401d-4a9c-b816-70384ce50576
📒 Files selected for processing (19)
adaptations/Yamaha_SY22.pyadaptations/Yamaha_SY35.pyadaptations/Yamaha_SY55.pyadaptations/Yamaha_SY77.pyadaptations/Yamaha_SY85.pyadaptations/Yamaha_TG33.pyadaptations/Yamaha_TG500.pyadaptations/Yamaha_TG77.pyadaptations/testData/Yamaha_SY22_SY35_TG33/SY22A.syxadaptations/testData/Yamaha_SY22_SY35_TG33/TG33-p1.syxadaptations/testData/Yamaha_SY22_SY35_TG33/ext.pyadaptations/testData/Yamaha_SY55/sy55.syxadaptations/testData/Yamaha_SY55/sy55_1.syxadaptations/testData/Yamaha_SY55/sy55_presets.syxadaptations/testData/Yamaha_SY55/sy55ptch.syxadaptations/testData/Yamaha_SY85/SY85_test_bank.syxadaptations/testData/Yamaha_TG500/TG500_test_bank.syxadaptations/yamaha/README_SY_TG_SYSEX.mdadaptations/yamaha/Yamaha_SY_TG_common.py
✅ Files skipped from review due to trivial changes (5)
- adaptations/testData/Yamaha_SY55/sy55.syx
- adaptations/testData/Yamaha_SY22_SY35_TG33/TG33-p1.syx
- adaptations/testData/Yamaha_SY55/sy55_presets.syx
- adaptations/testData/Yamaha_SY55/sy55ptch.syx
- adaptations/testData/Yamaha_SY85/SY85_test_bank.syx
🚧 Files skipped from review as they are similar to previous changes (1)
- adaptations/Yamaha_TG77.py
Owner
|
Release-readiness follow-up (no fixes applied here):
All eight current suites pass on Python 3.12 (186 tests total); the above cases are not covered. The earlier Enum membership failures on Python 3.10 are not a current 3.12 release blocker. SY77 hardware testing reported by the author should not be generalized to the whole family. |
Owner
|
That was codex 5.6 - I will have a look at this later to see if we need to harden some of the non-77 adaptations before including them in the official release. Thanks for the contribution! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Here's a follow up to the recent discussion about the Yamaha SY/TG series. I've implemented adaptations based on a common abstraction class.
Implementation Status:
See
adaptations/yamaha/Yamaha_SY_TG_common.pyfor implementation notes. The adaptation abstraction class should cover sufficient functionality of this series of synths. The quirks/differences that exist in each of the models can be implemented as overrides.adaptations/yamaha/README_SY_TG_SYSEX.mdsummarises the key sysex parameters used for these adaptations.Summary by CodeRabbit