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

Handle tray eject/load via smc #1564

Open
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

BiatuAutMiahn
Copy link

@BiatuAutMiahn BiatuAutMiahn commented Nov 28, 2023

As per #1563, allow user to specify -no_dvd to clear the dvd_path on startup.

@dracc
Copy link
Contributor

dracc commented Nov 28, 2023 via email

@BiatuAutMiahn
Copy link
Author

BiatuAutMiahn commented Nov 28, 2023

-dvd_path "" did not clear the path in my testing.

@GXTX
Copy link
Contributor

GXTX commented Nov 28, 2023

-no-dvd I'd assume we aren't attaching the drive (which could be useful for developers), not that we're claiming there's no disc inserted.

@BiatuAutMiahn
Copy link
Author

BiatuAutMiahn commented Nov 28, 2023

-no-dvd I'd assume we aren't attaching the drive (which could be useful for developers), not that we're claiming there's no disc inserted.

It's only clearing the path. Maybe I should implement -no_dvd_drive for testing as you suggested?

    if (strlen(dvd_path) > 0) {
        // Allow clearing the dvd path on boot.
        for (int i = 1; i < argc; i++) {
            if (argv[i] && strcmp(argv[i], "-no_dvd") == 0) {
                argv[i] = NULL;
                dvd_path = "";
                xemu_settings_set_string(&g_config.sys.files.dvd_path, "");
                break;
            }
        }
    }

@BiatuAutMiahn
Copy link
Author

BiatuAutMiahn commented Nov 28, 2023

How's this?

    bool no_dvd_drive = false;
    bool no_dvd = false;

    // debug only.
#ifdef DEBUG_XEMU_C
    for (int i = 1; i < argc; i++) {
        if (argv[i] && strcmp(argv[i], "-no_dvd_drive") == 0) {
            argv[i] = NULL;
            no_dvd_drive = true;
            break;
        }
    }
#endif

    const char *dvd_path = g_config.sys.files.dvd_path;
    if (!no_dvd_drive) {
        // Allow clearing the dvd path on boot.
        for (int i = 1; i < argc; i++) {
            if (argv[i] && strcmp(argv[i], "-no_dvd") == 0) {
                argv[i] = NULL;
                no_dvd = true;
                dvd_path = "";
                xemu_settings_set_string(&g_config.sys.files.dvd_path, "");
                break;
            }
        }
        if (!no_dvd) {
            // Allow overriding the dvd path from command line
            for (int i = 1; i < argc; i++) {
                if (argv[i] && strcmp(argv[i], "-dvd_path") == 0) {
                    argv[i] = NULL;
                    if (i < argc - 1 && argv[i+1]) {
                        dvd_path = argv[i+1];
                        argv[i+1] = NULL;
                    }
                    break;
                }
            }
            if (strlen(dvd_path) > 0) {
                if (xemu_check_file(dvd_path) || strcmp(dvd_path, hdd_path) == 0) {
                    char *msg = g_strdup_printf("Failed to open DVD image file '%s'. Please check machine settings.", dvd_path);
                    xemu_queue_error_message(msg);
                    g_free(msg);
                    dvd_path = "";
                }
            }
        }

        // Always populate DVD drive. If disc path is the empty string, drive is
        // connected but no media present.
        fake_argv[fake_argc++] = strdup("-drive");
        char *escaped_dvd_path = strdup_double_commas(dvd_path);
        fake_argv[fake_argc++] = g_strdup_printf("index=1,media=cdrom,file=%s",
            escaped_dvd_path);
        free(escaped_dvd_path);
    }

@mborgerson
Copy link
Member

mborgerson commented Nov 28, 2023

I thought an eject boot option was going to be added. This is different than starting with no dvd in the tray. -dvd_path "" should just load with no dvd in the tray (if it doesn't then it's a bug and should also be fixed).

-Reverts -no_dvd arg
-Adds eject/load tray handling to xbox smc.
-Adds -eject_after_boot arg
-Adds smc handling of eject_after_handling
@BiatuAutMiahn
Copy link
Author

BiatuAutMiahn commented Nov 28, 2023

I thought an eject boot option was going to be added. This is different than starting with no dvd in the tray. -dvd_path "" should just load with no dvd in the tray (if it doesn't then it's a bug and should also be fixed).

I think this is what you had in mind

Edit:
also the -dvd_path bit is bugged. If any args follow the -dvd_path, they get appended to the dvd_path string. However the "" works fine just doesn't update the config.

@BiatuAutMiahn BiatuAutMiahn changed the title Add -no_dvd parameter Handle tray eject/load. Nov 28, 2023
@BiatuAutMiahn BiatuAutMiahn changed the title Handle tray eject/load. Handle tray eject/load via smc Nov 28, 2023
Copy link
Contributor

@antangelo antangelo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please run clang-format on the code as per https://xemu.app/docs/dev/#contributing

hw/xbox/smbus_xbox_smc.c Outdated Show resolved Hide resolved
hw/xbox/smbus_xbox_smc.c Outdated Show resolved Hide resolved
hw/xbox/smbus_xbox_smc.c Outdated Show resolved Hide resolved
hw/xbox/smbus_xbox_smc.c Outdated Show resolved Hide resolved
hw/xbox/smbus_xbox_smc.c Show resolved Hide resolved
@antangelo antangelo self-assigned this May 25, 2024
hw/xbox/smbus_xbox_smc.c Show resolved Hide resolved
hw/xbox/smbus_xbox_smc.c Show resolved Hide resolved
hw/xbox/smbus_xbox_smc.c Outdated Show resolved Hide resolved
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.

5 participants