joan2937 / lg

Linux C libraries and Python modules for manipulating GPIO
The Unlicense
57 stars 20 forks source link

Bias #4

Closed waveform80 closed 2 years ago

waveform80 commented 3 years ago

While playing around with an integration layer for lgpio in gpio-zero, I noticed it wasn't possible to set the pulls on input pins. It appears later versions of the linux/gpio header include bias-related constants that permit this, and that's the main purpose of this patch which adds these constants to lgpio (and the relevant documentation for them to lgpio and rgpio).

However, given that this would make lg unable to compile on Raspbian Buster, it may be worth holding back on considering this patch until Bullseye's release? (I can always add it as a temporary patch to any packaging in the meantime).

It's probably also worth considering that it appears this ABI has (already!) been deprecated (see comments at the link above, and the V2 ABI constants at the top of the current header). Though I've yet to see the V2 ABI in the wild yet, so presumably one should target the V1 ABI for the immediate future and worry about this deprecation later!

It may also be worth considering the naming of the constants. I've gone with "consistency with the linux/gpio headers", but this does lead some some slightly odd grammar in things like "LG_GPIO_IS_BIAS_DISABLE". I'd be happy to revise the patch if you'd prefer alternative names?

I've split the PR into three commits: the first tweaks the constants themselves (and thus is the only code change). The second is purely documentation modifications, and the third is the trivial regeneration of the man-pages (from running DOC/makedoc).

joan2937 commented 3 years ago

I'd like to keep the master branch broadly aligned with the capabilities of Debian stable.

Perhaps a testing branch would suit your immediate needs?

I'm not too concerned about the naming of constants. My only driver would be stability, i.e. once a name has been used it forms part of the published interface and shouldn't be changed without good reason.

waveform80 commented 3 years ago

I'd like to keep the master branch broadly aligned with the capabilities of Debian stable.

Yup, figured that might be the case :)

Perhaps a testing branch would suit your immediate needs?

No need for a branch for my purposes - I'm quite happy maintaining this as a patch in the Ubuntu packaging for now. My main concern is making sure that whatever I patch in won't disagree with the names that eventually make it into master.

I'm not too concerned about the naming of constants. My only driver would be stability, i.e. once a name has been used it forms part of the published interface and shouldn't be changed without good reason.

If you're happy with the constant names as they are in the PR, I'll just add this as a patch locally and we could leave this PR open until such time as Debian stable's compatible with it (presumably later this year when bullseye becomes stable)? If anything winds up conflicting with it in the meantime, I can rebase it to keep it fresh.

joan2937 commented 2 years ago

This has probably been overtaken by the change to gpiochip API 2 (a forced change as it seems to have broken gpiochip API 1).

As part of the API 2 change I have added the pull constants to the various modules.

Incorporated in lg release v0.2

waveform80 commented 2 years ago

Ah, brilliant -- I'll get working on bumping the packaging. Thanks!