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

netdev: fix return value and precondition doc #10101

Merged
merged 1 commit into from
Oct 3, 2018

Conversation

miri64
Copy link
Member

@miri64 miri64 commented Oct 2, 2018

Contribution description

While reviewing #9942 I noticed that the documentation on the netdev
driver API is unclear and in some cases outright contradicting itself:

@return              number of bytes used from @p value
@return              `< 0` on error, 0 on success

IMHO this is unacceptable for such a central API where communication

This fixes a few things and also clarifies preconditions:

  • Specifies negative errnos clearly so all drivers return the same
    when required
  • Re-iterates parameter preconditions and special cases within the
    parameter documentation itself (might also help towards Misleading API in netdev driver #9805?)
  • Fixes contradictions within return value documentation
  • Adds missing parameter documentation on init().

Testing procedure

Compile documentation (make doc) and see if everything is alright in doc/doxygen/html/structnetdev__driver.html.

Issues/PRs references

Noticed in #9942. Might help clarify #9805.

@miri64 miri64 added Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Area: network Area: Networking Area: doc Area: Documentation Area: drivers Area: Device drivers labels Oct 2, 2018
drivers/include/net/netdev.h Show resolved Hide resolved
drivers/include/net/netdev.h Show resolved Hide resolved
drivers/include/net/netdev.h Show resolved Hide resolved
* @return `-ENOTSUP` if @p opt is not configurable for the
* device
* @return `-EINVAL` if @p value is invalid an invalid value
* for @p opt
Copy link
Member

Choose a reason for hiding this comment

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

This sentence doesn't parse for me.

Copy link
Member Author

Choose a reason for hiding this comment

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

For me neither. Only drank weak coffee before that so that might be the cause ;-).

@miri64
Copy link
Member Author

miri64 commented Oct 2, 2018

(Just noticed: this is PR #10101 🎉)

Copy link
Member

@bergzand bergzand left a comment

Choose a reason for hiding this comment

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

ACK, please squash

@bergzand bergzand added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Oct 3, 2018
While reviewing RIOT-OS#9942 I noticed that the documentation on the netdev
driver API is unclear and in some cases outright contradicting itself:

> ```
> @return              number of bytes used from @p value
> @return              `< 0` on error, 0 on success
> ```

IMHO this is unacceptable for such a central API where communication

This fixes a few things and also clarifies preconditions:

- Specifies negative `errno`s clearly so all drivers return the same
  when required
- Re-iterates parameter preconditions and special cases within the
  parameter documentation itself (might also help towards RIOT-OS#9805?)
- Fixes contradictions within return value documentation
- Adds missing parameter documentation on `init()`.
@miri64 miri64 force-pushed the netdev/doc/fix-return-asserts branch from 679f352 to aba75be Compare October 3, 2018 14:10
@miri64
Copy link
Member Author

miri64 commented Oct 3, 2018

Squashed

@miri64 miri64 merged commit 4b88418 into RIOT-OS:master Oct 3, 2018
@miri64 miri64 deleted the netdev/doc/fix-return-asserts branch October 3, 2018 15:15
@jia200x jia200x added this to the Release 2018.10 milestone Oct 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: doc Area: Documentation Area: drivers Area: Device drivers Area: network Area: Networking CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants