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

Jump to bottom

usb_modeswitch: fixes #60981

Merged
merged 1 commit into from Jan 14, 2020
Merged

usb_modeswitch: fixes #60981

merged 1 commit into from Jan 14, 2020

Conversation

thefloweringash
Copy link
Member

@thefloweringash thefloweringash commented May 5, 2019 •
edited

Motivation for this change

usb_modeswitch currently isn't working #60878

This is unfortunately quite an extensive change, mostly redoing the packaging of usb-modeswitch itself. Because of the size of the change, I'd like a review from the nixpkgs packaging experts before attempting to upstream this. I have successfully tested this with an "LG L-02C LTE".

This has the following goals:

  1. Make DESTDIR and PREFIX work as per usual. Currently PREFIX isn't supported due to many hardcoded paths.

  2. Remove hardcoded paths to dependencies and rely on the PATH variable instead.

  3. Allow configuration of which init systems to use, instead of checking the state of the build machine.

Still TODO:

  • Fix kernel module probing

  • New default PREFIX changes the result of make install. Either change the readme, or restore the existing behavior.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@ofborg ofborg bot requested a review from peterhoeg May 5, 2019 11:22
@nyanloutre
Copy link
Member

nyanloutre commented May 10, 2019 •
edited

What do you mean by "Fix kernel module probing" ?

Answer from Matrix: "there's a section in the dispatcher that checks for potential kernel features by looking at /lib/modules/$(uname -r). this won't work as is, but I don't have a plan for how to fix it yet"

@Yarny0
Copy link
Contributor

Yarny0 commented May 11, 2019

As I'm also suffering from broken usb-modeswitch and I have hardware to test this pull-request, I'm offering this review (per https://nixos.org/nixpkgs/manual/#reviewing-contributions-package-updates and https://nixos.org/nixpkgs/manual/#reviewing-contributions-module-updates). Note that this is a partial review as I can't follow all the changes that the patch file of brings.

Reviewed points (package update)
  • package name fits guidelines (no change)
  • package version fits guidelines (no change)
  • package build on x86_64-linux
  • executables tested on x86_64-linux
  • all depending packages build
Reviewed points (module update)
  • changes are backward compatible
  • removed options are declared with mkRemovedOptionModule (not applicable)
  • changes that are not backward compatible are documented in release notes (not applicable)
  • module tests succeed on ARCHITECTURE
  • options types are appropriate (no change)
  • options description is set (no change)
  • options example is provided (no change)
  • documentation affected by the changes is updated
Comments
  • I tested this module with a Huawei K3765: It works as expected (but didn't before applying this pull request)
    • A few seconds after plugging in the device, the system log told me that the usb modeswitching service is started.
    • Devices /dev/ttyUSB{0,1,2} appeared.
    • I used picocom and some AT-commands to query my provider's balance.
  • Tested after rebasing onto current nixos-unstable channel (bc94dcf).
  • For what it's worth: I expected ModemManager to pick up the device, but ModemManager didn't auto-start -- I had to start it manually with systemctl start ModemManager. This is possibly due to 3f9ac24 .
  • Notifying usb-modeswitch co-maintainer: @MarcWeber

@nyanloutre
Copy link
Member

Tested successfully with the Logitech G920 racing wheel

@nyanloutre
Copy link
Member

Is this still WIP ?

@nyanloutre
Copy link
Member

What should be done before it can be merged ? I am using it for a few months now without issues

@nyanloutre
Copy link
Member

The patch is not applying anymore on master

@nyanloutre
Copy link
Member

I updated the patch in this commit : nyanloutre@b092c46

@peterhoeg peterhoeg changed the title [WIP] usb_modeswitch usb_modeswitch: fixes Jan 14, 2020
@peterhoeg peterhoeg merged commit 41d333e into NixOS:master Jan 14, 2020
@peterhoeg
Copy link
Member

Thank you all for your patience.

@nyanloutre
Copy link
Member

Hydra build is failing because my updated patch didn't get merged.

I made a PR: #77695

@michalrus
Copy link
Member

Works for me, too!

@michalrus michalrus mentioned this pull request May 10, 2020
Closed
@thefloweringash thefloweringash deleted the usb-modeswitch branch April 26, 2021 02:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@peterhoeg peterhoeg Awaiting requested review from peterhoeg

Assignees
No one assigned
Labels
2.status: work-in-progress 6.topic: nixos 8.has: module (update) 10.rebuild-darwin: 0 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@thefloweringash @nyanloutre @Yarny0 @peterhoeg @michalrus @samueldr