[libgpiod,v2,0/6] documentation and other minor tweaks

The bulk of this series is the documentation tweak pass I promised when
reviewing other v2 changes.  The rest is a collection of minor things
I tripped over in the process.

The first is expanding the usage of size_t within the library to cover
the occasions loop variables should also be size_t.  A noteable exception
is for offsets, but those are defined as u32 in the uAPI and unsigned int
in the libgpiod API, so adding another intermediate type seemed like a
bad idea.

The second and fifth are function renaming for consistency.

The third and fourth are just renaming variables for clarity.

The sixth is the actual documentation tweaks.

The changes and reasoning behind them is as follows:

Fix a few typos.

Add "::" to symbols doxygen links where missing.

Use "\p" to refer to parameters.

Fix space before tabs (triggers a checkpatch warning even if the line
isn't changed).
Indentation uses tabs then spaces throughout.

Change "@param offset Offset of the line" to "@param offset The offset of
the line" to avoid checkpatch repeated word warnings :-/.

Use "Get" to describe getters, rather than "Read".

Use "function" not "routine".

Drop "GPIO" from descriptions where it doesn't add anything.

Drop use of current/currently as it is clear it couldn't be otherwise,
and adding "current" just makes this reader wonder how to access
non-current.

Some rewording to improve clarity.

Add some @notes to cover misconceptions or questions I frequently see.

The API is all about chips and lines.
Recognise that "offset" is an identifier for a line, just as "name"
could be. So don't use "offset" as a synonymous placeholder for line - use
line.

Use "num_lines" instead of "num_offsets" or "num_values".
offsets and values are just attributes of lines, so num_offsets =
num_values = num_lines, and num_lines is always appropriate, independent
of which set of attributes are being described.

Use of the definite and indefinite article:

In general, where something is not unique it is described using the
indefinite article, "a", but if it is unique, including where some
selection has already been performed, then use the definite article,
"the".

Only use "this" to emphasise a specific item selected from a set,
such as when referring to "this function".
Generally "the" is better, and avoids any possible confusion with C++
this.

Generally use the indefinite article for @brief descriptions.
e.g. "The function does something to a thing."
rather than "The function does something to the thing.", as it is up to
the caller to make the selection as to which definite thing to call the
function on.

For containers, an attribute of the contained element is definite, but the
element itself is indefinite:
"Clear the edge detection override for a line."

For snapshots, like line_info, the "line" becomes definite as the act of
taking the snapshot selects the line.
So "Get the name of the line."

The @param and @return use the definite article as they either identify
the article, or refer to a specific article, not the generic operation
of the function like @brief.


Do NOT ask me to split those out into separate patches ;-).


I realise this aimed at a moving target, so I'm rushing this out a little.
The commit that this is based off is indicated below - the current
next/libgpiod-2.0 head at time of writing.

Cheers,
Kent.

Kent Gibson (6):
  treewide: use size_t for loop variable where limit is size_t
  API: rename gpiod_request_config_get_num_offsets to
    gpiod_request_config_get_num_lines to match line_request pattern
  line-config: rename off to idx
  line-config: rename num_values to num_lines
  line-request: rename wait and read functions
  doc: API documentation tweaks

 include/gpiod.h              | 712 +++++++++++++++++++----------------
 lib/edge-event.c             |   2 +-
 lib/line-config.c            |  36 +-
 lib/line-info.c              |   2 +-
 lib/line-request.c           |  10 +-
 lib/request-config.c         |  26 +-
 tests/tests-edge-event.c     |  38 +-
 tests/tests-line-request.c   |   2 +-
 tests/tests-request-config.c |   8 +-
 tools/gpioget.c              |   4 +-
 tools/gpioinfo.c             |   4 +-
 tools/gpiomon.c              |   4 +-
 tools/gpioset.c              |   6 +-
 13 files changed, 449 insertions(+), 405 deletions(-)


base-commit: 6e15b78d6e9c956c295c755aed793ffd963b1c53

On Fri, Mar 11, 2022 at 8:39 AM Kent Gibson <warthog618@gmail.com> wrote:
>
> The bulk of this series is the documentation tweak pass I promised when
> reviewing other v2 changes.  The rest is a collection of minor things
> I tripped over in the process.
>
> The first is expanding the usage of size_t within the library to cover
> the occasions loop variables should also be size_t.  A noteable exception
> is for offsets, but those are defined as u32 in the uAPI and unsigned int
> in the libgpiod API, so adding another intermediate type seemed like a
> bad idea.
>
> The second and fifth are function renaming for consistency.
>
> The third and fourth are just renaming variables for clarity.
>
> The sixth is the actual documentation tweaks.
>
> The changes and reasoning behind them is as follows:
>
> Fix a few typos.
>
> Add "::" to symbols doxygen links where missing.
>
> Use "\p" to refer to parameters.
>
> Fix space before tabs (triggers a checkpatch warning even if the line
> isn't changed).
> Indentation uses tabs then spaces throughout.
>
> Change "@param offset Offset of the line" to "@param offset The offset of
> the line" to avoid checkpatch repeated word warnings :-/.
>
> Use "Get" to describe getters, rather than "Read".
>
> Use "function" not "routine".
>
> Drop "GPIO" from descriptions where it doesn't add anything.
>
> Drop use of current/currently as it is clear it couldn't be otherwise,
> and adding "current" just makes this reader wonder how to access
> non-current.
>
> Some rewording to improve clarity.
>
> Add some @notes to cover misconceptions or questions I frequently see.
>
> The API is all about chips and lines.
> Recognise that "offset" is an identifier for a line, just as "name"
> could be. So don't use "offset" as a synonymous placeholder for line - use
> line.
>
> Use "num_lines" instead of "num_offsets" or "num_values".
> offsets and values are just attributes of lines, so num_offsets =
> num_values = num_lines, and num_lines is always appropriate, independent
> of which set of attributes are being described.
>
> Use of the definite and indefinite article:
>
> In general, where something is not unique it is described using the
> indefinite article, "a", but if it is unique, including where some
> selection has already been performed, then use the definite article,
> "the".
>
> Only use "this" to emphasise a specific item selected from a set,
> such as when referring to "this function".
> Generally "the" is better, and avoids any possible confusion with C++
> this.
>
> Generally use the indefinite article for @brief descriptions.
> e.g. "The function does something to a thing."
> rather than "The function does something to the thing.", as it is up to
> the caller to make the selection as to which definite thing to call the
> function on.
>
> For containers, an attribute of the contained element is definite, but the
> element itself is indefinite:
> "Clear the edge detection override for a line."
>
> For snapshots, like line_info, the "line" becomes definite as the act of
> taking the snapshot selects the line.
> So "Get the name of the line."
>
> The @param and @return use the definite article as they either identify
> the article, or refer to a specific article, not the generic operation
> of the function like @brief.
>
>
> Do NOT ask me to split those out into separate patches ;-).
>
>
> I realise this aimed at a moving target, so I'm rushing this out a little.
> The commit that this is based off is indicated below - the current
> next/libgpiod-2.0 head at time of writing.
>

Hi Kent!

Thanks for taking the time. I'm mostly ok with those changes,
especially the documentation as I don't really know better (ESL). I
think I may change a couple minor things in the first patches but
nothing big. As far as patch attribution goes - do you want me to
apply it to next/libgpiod-2.0 now and squash it later with your name
mentioned in the commit message or do you want me to queue them for
later once the C++ and Python parts are in as well?

Bart

> Cheers,
> Kent.
>
> Kent Gibson (6):
>   treewide: use size_t for loop variable where limit is size_t
>   API: rename gpiod_request_config_get_num_offsets to
>     gpiod_request_config_get_num_lines to match line_request pattern
>   line-config: rename off to idx
>   line-config: rename num_values to num_lines
>   line-request: rename wait and read functions
>   doc: API documentation tweaks
>
>  include/gpiod.h              | 712 +++++++++++++++++++----------------
>  lib/edge-event.c             |   2 +-
>  lib/line-config.c            |  36 +-
>  lib/line-info.c              |   2 +-
>  lib/line-request.c           |  10 +-
>  lib/request-config.c         |  26 +-
>  tests/tests-edge-event.c     |  38 +-
>  tests/tests-line-request.c   |   2 +-
>  tests/tests-request-config.c |   8 +-
>  tools/gpioget.c              |   4 +-
>  tools/gpioinfo.c             |   4 +-
>  tools/gpiomon.c              |   4 +-
>  tools/gpioset.c              |   6 +-
>  13 files changed, 449 insertions(+), 405 deletions(-)
>
>
> base-commit: 6e15b78d6e9c956c295c755aed793ffd963b1c53
> --
> 2.35.1
>

On Mon, Mar 14, 2022 at 09:31:27AM +0100, Bartosz Golaszewski wrote:
> On Fri, Mar 11, 2022 at 8:39 AM Kent Gibson <warthog618@gmail.com> wrote:
> >

[snip]

> 
> Hi Kent!
> 
> Thanks for taking the time. I'm mostly ok with those changes,
> especially the documentation as I don't really know better (ESL). I
> think I may change a couple minor things in the first patches but
> nothing big. As far as patch attribution goes - do you want me to
> apply it to next/libgpiod-2.0 now and squash it later with your name
> mentioned in the commit message or do you want me to queue them for
> later once the C++ and Python parts are in as well?
> 

It would certainly make sense to apply the API changes before the
binding changes, to avoid having to update the bindings later, even
if the changes are minor.

Whatever works for you.

Cheers,
Kent.

On Fri, Mar 11, 2022 at 8:39 AM Kent Gibson <warthog618@gmail.com> wrote:
>
> The bulk of this series is the documentation tweak pass I promised when
> reviewing other v2 changes.  The rest is a collection of minor things
> I tripped over in the process.
>
> The first is expanding the usage of size_t within the library to cover
> the occasions loop variables should also be size_t.  A noteable exception
> is for offsets, but those are defined as u32 in the uAPI and unsigned int
> in the libgpiod API, so adding another intermediate type seemed like a
> bad idea.
>
> The second and fifth are function renaming for consistency.
>
> The third and fourth are just renaming variables for clarity.
>
> The sixth is the actual documentation tweaks.
>
> The changes and reasoning behind them is as follows:
>
> Fix a few typos.
>
> Add "::" to symbols doxygen links where missing.
>
> Use "\p" to refer to parameters.
>
> Fix space before tabs (triggers a checkpatch warning even if the line
> isn't changed).
> Indentation uses tabs then spaces throughout.
>
> Change "@param offset Offset of the line" to "@param offset The offset of
> the line" to avoid checkpatch repeated word warnings :-/.
>
> Use "Get" to describe getters, rather than "Read".
>
> Use "function" not "routine".
>
> Drop "GPIO" from descriptions where it doesn't add anything.
>
> Drop use of current/currently as it is clear it couldn't be otherwise,
> and adding "current" just makes this reader wonder how to access
> non-current.
>
> Some rewording to improve clarity.
>
> Add some @notes to cover misconceptions or questions I frequently see.
>
> The API is all about chips and lines.
> Recognise that "offset" is an identifier for a line, just as "name"
> could be. So don't use "offset" as a synonymous placeholder for line - use
> line.
>
> Use "num_lines" instead of "num_offsets" or "num_values".
> offsets and values are just attributes of lines, so num_offsets =
> num_values = num_lines, and num_lines is always appropriate, independent
> of which set of attributes are being described.
>
> Use of the definite and indefinite article:
>
> In general, where something is not unique it is described using the
> indefinite article, "a", but if it is unique, including where some
> selection has already been performed, then use the definite article,
> "the".
>
> Only use "this" to emphasise a specific item selected from a set,
> such as when referring to "this function".
> Generally "the" is better, and avoids any possible confusion with C++
> this.
>
> Generally use the indefinite article for @brief descriptions.
> e.g. "The function does something to a thing."
> rather than "The function does something to the thing.", as it is up to
> the caller to make the selection as to which definite thing to call the
> function on.
>
> For containers, an attribute of the contained element is definite, but the
> element itself is indefinite:
> "Clear the edge detection override for a line."
>
> For snapshots, like line_info, the "line" becomes definite as the act of
> taking the snapshot selects the line.
> So "Get the name of the line."
>
> The @param and @return use the definite article as they either identify
> the article, or refer to a specific article, not the generic operation
> of the function like @brief.
>
>
> Do NOT ask me to split those out into separate patches ;-).
>
>
> I realise this aimed at a moving target, so I'm rushing this out a little.
> The commit that this is based off is indicated below - the current
> next/libgpiod-2.0 head at time of writing.
>
> Cheers,
> Kent.
>
> Kent Gibson (6):
>   treewide: use size_t for loop variable where limit is size_t
>   API: rename gpiod_request_config_get_num_offsets to
>     gpiod_request_config_get_num_lines to match line_request pattern
>   line-config: rename off to idx
>   line-config: rename num_values to num_lines
>   line-request: rename wait and read functions
>   doc: API documentation tweaks

I applied patches 1, 3, 5 and most of 6. I think we should not apply 2
and 4 as explained in their respective threads.

Do you want to send a follow up to the doco changes to the rejected
parts of patch 6 but without changing the num_values name?

Bart

>
>  include/gpiod.h              | 712 +++++++++++++++++++----------------
>  lib/edge-event.c             |   2 +-
>  lib/line-config.c            |  36 +-
>  lib/line-info.c              |   2 +-
>  lib/line-request.c           |  10 +-
>  lib/request-config.c         |  26 +-
>  tests/tests-edge-event.c     |  38 +-
>  tests/tests-line-request.c   |   2 +-
>  tests/tests-request-config.c |   8 +-
>  tools/gpioget.c              |   4 +-
>  tools/gpioinfo.c             |   4 +-
>  tools/gpiomon.c              |   4 +-
>  tools/gpioset.c              |   6 +-
>  13 files changed, 449 insertions(+), 405 deletions(-)
>
>
> base-commit: 6e15b78d6e9c956c295c755aed793ffd963b1c53
> --
> 2.35.1
>