Skip to content

Added some reasonable defaults to LIRC's module #53795

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

Closed
wants to merge 6 commits into from

Conversation

alex-ameen
Copy link
Contributor

@alex-ameen alex-ameen commented Jan 11, 2019

Motivation for this change

After spending a few hours trying to install LIRC I realized that many of my issues were from a lack of default settings that could get the program up and running. This will allow many users to let LIRC run out of the box.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • [x ] 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 nox --run "nox-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.

@alex-ameen alex-ameen requested a review from infinisil as a code owner January 11, 2019 07:28
@GrahamcOfBorg GrahamcOfBorg added 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 labels Jan 11, 2019
@markuskowa
Copy link
Member

CC @ck3d


[lircmd]
uinput = False
nodaemon = False
Copy link
Member

@Mic92 Mic92 Jan 11, 2019

Choose a reason for hiding this comment

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

To bad this was not added in the first place. I would prefer having sane default options, that can be extended by extraOptions. Do you think users would need to extend the default options in the common case?

Copy link
Member

Choose a reason for hiding this comment

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

I think for now this is alright, can always add that later

Copy link
Member

Choose a reason for hiding this comment

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

agreed.

@ck3d
Copy link
Contributor

ck3d commented Jan 12, 2019

My configuration file looks as follows:

[lircd]
driver = ftdi
device = product=0x6015

I set none of the default values in my configuration file without any problems. Could you please specify which problems you have?

@infinisil
Copy link
Member

@BadDecisionsAlex We should strive to keep the default config as small as possible. If @ck3d's config works then that is preferred.

@alex-ameen
Copy link
Contributor Author

My configuration file looks as follows:

[lircd]
driver = ftdi
device = product=0x6015

I set none of the default values in my configuration file without any problems. Could you please specify which problems you have?

Sorry for the late reply, I've been swamped.
Without the default settings that I added LIRC was unable to access any of the drivers which it is normally packaged with.

@GrahamcOfBorg GrahamcOfBorg added 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux and removed 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-linux: 1-10 labels Feb 8, 2019
@ck3d
Copy link
Contributor

ck3d commented Feb 9, 2019

No problem.
To check that lircd find its drivers, call following command:

lircd -H help

In my case, everything is found. The default, ftdi and 30 more drivers.

I think the main reason is the missing device setting. Please give following config a try:

[lircd]
device = /dev/lirc0

or set it as extraArguments:
extraArguments = [ "--device=/dev/lirc0" ];

I had a look into lirc sources and it showed that the default device of the lircd default driver is /dev/lirc/0 (defined as LIRC_DRIVER_DEVICE). If /dev/lirc0 is a better default, we should think about patching lirc.

As alternative we could provide two NixOS options for driver and device.

Fixed revisions:
1) Uses `pname`.
2) URL uses version variable in path. Data type for `url` was changed from path to string.
The last commit mistakenly reverted the URL to "old" again. The change has been re-applied.
@ofborg ofborg bot added 10.rebuild-linux: 1-10 and removed 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux labels May 27, 2019
@alex-ameen
Copy link
Contributor Author

Sorry I did not mean to revive this. This has been sitting without interest for a while. I'm closing it.

@alex-ameen alex-ameen closed this May 27, 2019
@Janik-Haag Janik-Haag added the 12. first-time contribution This PR is the author's first one; please be gentle! label Jun 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 12. first-time contribution This PR is the author's first one; please be gentle!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants