这是indexloc提供的服务,不要输入任何密码
Skip to content

Conversation

@devin-ai-integration
Copy link
Contributor

Fix C++ MIDI-CI Profile Parsing in PROFILE_INQUIRY_REPLY

Fixes the issue where PROFILE_INQUIRY_REPLY messages created empty profile vectors instead of parsing the actual profile data from the sysex message, implementing the C++ equivalent of Kotlin's CIRetrieval.midiCIGetProfileSet() functionality.

Problem

The C++ implementation in Messenger::process_input() was creating empty std::vector<std::vector<uint8_t>> for both enabled and disabled profiles when processing PROFILE_INQUIRY_REPLY messages, instead of parsing the actual profile data from the sysex bytes. This prevented proper profile inspection message processing.

Solution

Changes Made:

  • Modified PROFILE_INQUIRY_REPLY case in Messenger::process_input() to extract profile data from bytes 13+ of the sysex message
  • Reused existing ProfileReply::deserialize() method for proper parsing of enabled and disabled profiles
  • Added size check to ensure minimum data length before parsing
  • Fixed PROPERTY_SUBSCRIPTION_REPLY to properly extract body data from the message

Implementation Details:

  • Extracts profile data starting from byte 13 (after the common MIDI-CI header)
  • Uses the existing ProfileReply::deserialize() method which correctly implements the parsing logic that matches the Kotlin reference
  • The deserialize method properly handles:
    • Reading number of enabled profiles from first byte (with 7-bit masking)
    • Extracting 5-byte profile IDs for each enabled profile
    • Reading number of disabled profiles after enabled ones
    • Extracting 5-byte profile IDs for each disabled profile

Testing

  • ✅ Build passes without compilation errors
  • ✅ All profile-related tests pass (8/8 tests)
  • ✅ General MIDI-CI tests pass
  • ✅ Parsing logic matches Kotlin CIRetrieval.midiCIGetProfileSet() implementation

Verification

The fix ensures that profile data is properly extracted and parsed from MIDI-CI messages, enabling correct profile inspection and further profile-related message processing that was previously broken due to empty profile vectors.

Link to Devin run: https://app.devin.ai/sessions/e63842341c21412c86cc2f8a5a694d2f
Requested by: Atsushi Eno (atsushieno@gmail.com)

- Extract profile data from sysex message bytes 13+
- Use existing ProfileReply::deserialize() method for parsing
- Matches Kotlin CIRetrieval.midiCIGetProfileSet() functionality
- Fixes empty profile vectors issue in process_input()
- Also fix PROPERTY_SUBSCRIPTION_REPLY to extract body data properly

Co-Authored-By: Atsushi Eno <atsushieno@gmail.com>
@devin-ai-integration
Copy link
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@atsushieno atsushieno merged commit e95ef94 into main Jun 16, 2025
@devin-ai-integration
Copy link
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

1 similar comment
@devin-ai-integration
Copy link
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@atsushieno atsushieno deleted the devin/1750084122-fix-cpp-message-deserialization branch June 19, 2025 05:59
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