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

[gpio] Update README.md #9899

Merged
merged 3 commits into from
Feb 17, 2021
Merged

[gpio] Update README.md #9899

merged 3 commits into from
Feb 17, 2021

Conversation

SebBoulet
Copy link
Contributor

Added instructions in case raspi-config is not available.

Comment on lines +32 to +33
ExecStart=
ExecStart=/usr/bin/pigpiod
Copy link
Member

Choose a reason for hiding this comment

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

Why are there two ExecStarts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because this is what the raspi-config script adds to the pigpiod config file and I copied verbatim (in case it matters).
https://github.com/RPi-Distro/raspi-config/blob/74885481d8f311ab3875b6601153132552b41d67/raspi-config#L1616

Copy link
Member

Choose a reason for hiding this comment

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

It seems an error to me. Can you test if it still works if you remove the empty ExecStart=?

Copy link
Member

Choose a reason for hiding this comment

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

It seems to be to override the default values. See https://bugzilla.redhat.com/show_bug.cgi?id=756787. So I guess it is to be sure no default settings are present. That would mean even if @SebBoulet would test this on his system and it works, it might not work for other people who have other configurations. So I would say this is legitimate use 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 didn't find that info when doing a quick search. Thanks!

Copy link
Member

@Hilbrand Hilbrand left a comment

Choose a reason for hiding this comment

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

LGTM

SebBoulet and others added 2 commits February 8, 2021 22:38
Added instructions in case raspi-config is not available.

Signed-off-by: Seb Boulet <SebBoulet@users.noreply.github.com>
Co-authored-by: Hilbrand Bouwkamp <hilbrand@h72.nl>
Signed-off-by: Seb Boulet <SebBoulet@users.noreply.github.com>
@openhab-bot
Copy link
Collaborator

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/gpio-new-gpio-binding-for-oh3/113943/16

Co-authored-by: Hilbrand Bouwkamp <hilbrand@h72.nl>
Signed-off-by: Seb Boulet <SebBoulet@users.noreply.github.com>
Copy link
Member

@Hilbrand Hilbrand left a comment

Choose a reason for hiding this comment

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

LGTM

@Hilbrand Hilbrand merged commit cb9a118 into openhab:main Feb 17, 2021
@Hilbrand Hilbrand added this to the 3.1 milestone Feb 17, 2021
K4ntir4n pushed a commit to K4ntir4n/openhab-addons that referenced this pull request Feb 17, 2021
…hab#9899)

Signed-off-by: Seb Boulet <SebBoulet@users.noreply.github.com>
themillhousegroup pushed a commit to themillhousegroup/openhab2-addons that referenced this pull request May 10, 2021
…hab#9899)

Signed-off-by: Seb Boulet <SebBoulet@users.noreply.github.com>
Signed-off-by: John Marshall <john.marshall.au@gmail.com>
thinkingstone pushed a commit to thinkingstone/openhab-addons that referenced this pull request Nov 7, 2021
…hab#9899)

Signed-off-by: Seb Boulet <SebBoulet@users.noreply.github.com>
marcfischerboschio pushed a commit to bosch-io/openhab-addons that referenced this pull request May 5, 2022
…hab#9899)

Signed-off-by: Seb Boulet <SebBoulet@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants