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

Adjust logic for supported vendor commands #434

Merged
merged 2 commits into from
Jul 5, 2024

Conversation

morio
Copy link
Collaborator

@morio morio commented Jul 3, 2024

There isn't any functional change in this fix, just a rearrangement with if else statements to address this issue: #432 Also a for loop wasn't initialized in src/ZuluSCSI.cpp, set the counter to 0.

There isn't any functional change in this fix, just a rearrangement
with if else statements to address this issue: #432
Also a for loop wasn't initialized in `src/ZuluSCSI.cpp`, set the
counter to 0.
@saybur
Copy link
Contributor

saybur commented Jul 4, 2024

Sorry, I wasn't being very helpful with my link in the issue. If that linked comment's intent is right, then I think the problem is actually here:

else if (((cfg->deviceType == S2S_CFG_OPTICAL) && scsiCDRomCommand()) ||

Where the CD-ROM handler is being called and resolves the command before the vendor handler is called:

else if (scsiVendorCommand())

@morio
Copy link
Collaborator Author

morio commented Jul 4, 2024

Ahh yeah, I see the issue. Thanks!
I could add a !scsiToolboxEnabled() to the three CD commands.
It probably make more sense to move

else if (scsiToolboxEnabled() && scsiToolboxCommand())
{
// already handled
}

into the scsi.c above the specific device handling.

The actual issue, as pointed out by @saybur,
here: #434 (comment)

was that `scsiCDRomCommand()` was handling CD-ROM specific vendor
commands before `scsiVendorCommand()`, where the Toolbox vendor commands
were being handled. When Apple quirks or Plextor vendor commands
were enabled and Toolbox was enabled, the CD-ROM vendor commands were
overriding the Toolbox commands.

This fix moves `scsiToolboxCommand()` into `scsi.c` above
scsiCDRomCommand to override the CD-ROM specific commands.
@aperezbios aperezbios merged commit c0d8027 into main Jul 5, 2024
2 checks passed
@aperezbios aperezbios deleted the fix/reorder-vendor-logic branch July 5, 2024 19:13
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