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

Fastboot device detection fix, Linux terminal option settings, logging improvements #247

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

zraexy
Copy link

@zraexy zraexy commented Sep 23, 2024

These commits resolve a couple of issues I ran into using PixelFlasher.

The fastboot issue was triggered by plugging in my Pixel on the same USB bus that my laptop fingerprint reader is on.

The shell/terminal changes were made to support using KDE Plasma and NixOS.
KDE konsole uses "-e" and "--workdir" for its options ("-e" has different argument splitting behavior in gnome-terminal).
NixOS doesn't have a global "/bin/bash".

Ideally there would be an error message if "Linux Shell" is not set, but I opted to instead just supply defaults because it will at least log an error. The current behavior for running commands doesn't even provide any notification that there is an issue. Additionally, even if "Linux Shell" is set, it doesn't mean that the default option arguments are correct for the specified shell, and I assume you don't want to force people to manually provide the options when using gnome-terminal.

Thank you for building this!

The fastboot cli utility returns a status of "fastboot" if the usb interface name is null.
It can only be non-null on Linux, and this theoretically should only happen with a phone running fastbootd, in which case it will be "fastbootd" instead.
The issue occurs due to a bug where reuse of the same struct and failure to reset the string causes the interface name for a different device on the same USB bus to be returned instead.
Since only devices in fastboot mode should be listed anyway, the solution is to not care about the status string and instead just look for the serial number.
Also, fastboot doesn't respect "-s" under the "devices" sub-command.
@badabing2005
Copy link
Owner

The shell/terminal changes were made to support using KDE Plasma and NixOS.
KDE konsole uses "-e" and "--workdir" for its options ("-e" has different argument splitting behavior in gnome-terminal).
NixOS doesn't have a global "/bin/bash".

Ideally there would be an error message if "Linux Shell" is not set, but I opted to instead just supply defaults because it will at least log an error. The current behavior for running commands doesn't even provide any notification that there is an issue. Additionally, even if "Linux Shell" is set, it doesn't mean that the default option arguments are correct for the specified shell, and I assume you don't want to force people to manually provide the options when using gnome-terminal.

The idea of Linux shell / File manager settings is simple, people know their own shell and parameters required to launch what they like how they like.

They set the entire command (with arguments) into the settings.
Simple as that, rather than split the command, arguments, directory ... into separate fields.
There is My Tools section for that, have you checked?
I don't want to overcrowd the settings window, which is already crowded enough.

I like your changes, specially the part about making it more platform agnostic, but having extra UI widgets for arguments / directory in settings window, I'd rather do without.

Thanks

@zraexy
Copy link
Author

zraexy commented Sep 24, 2024

I have removed the separate settings.

Currently, anyone who has explicitly specified a "Linux Shell" will have their setup break because the command option argument must now be manually included. Perhaps the correct thing to do would be to change the attribute name in the JSON config so that it becomes unset?

I have also removed the changes that make gnome-terminal the default in all cases, although, it does seem strange to default to using gnome-terminal when opening a shell in a given directory, but not to do so for launching ADB Shell & Scrcpy.

Use env to execute bash so that it can be automatically located on unconventional distributions.
"Linux Shell Command" now must include the command option argument (e.g. '--' or '-e').
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