[OA-syst-ref] Re: Proposal for Updating Doxygen Comments within OpenAMP for Consistent Documentation of Input Parameters

Hi Leino,

Thanks for this proposal, it's definitely a good roadmap from my perspective.

Few comment/question in line

Regards Arnaud

ST Restricted

-----Original Message----- From: Leino, Tammy via Openamp-system-reference <openamp-system- reference@lists.openampproject.org> Sent: Thursday, March 16, 2023 3:14 PM To: openamp-system-reference@lists.openampproject.org Subject: [OA-syst-ref] Proposal for Updating Doxygen Comments within OpenAMP for Consistent Documentation of Input Parameters

Hello All,

As per the discussion last week in the OpenAMP System Reference call, here is my proposal for updating the Doxygen comments for consistent documentation of input parameters. I would like to target these changes for the April 2023 release. I will be sending out another email with proposed changes for documenting data structures also targeting the April 2023 release. There are other changes I will be proposing for future releases, but these two proposals are what seem doable for April.

Overview:

1 - Isolate all Doxygen comments to header files. Doxygen comments are currently sporadically located between source and header files. Some routines have Doxygen comments in both places. Zephyr has Doxygen comments for all functions/data structures in header files (except for static/externed functions), so the proposal is to follow their example. 2 - Update Doxygen comment formatting to be consistent. 3 - Ensure all i/o parameters for APIs are documented.

Examples:

Following shows just one example of each suggestion.

#1 - Isolate all Doxygen comments to header files. ++++++++++++++++++++++++++++++++++++++ rpmsg_send_offchannel_raw() The Doxygen comments are found in both the header file and the source file. The formatting in the source file is proper, and is given priority when the code is built, so it is used and displayed properly in the html. Recommend to move the formatting to the header file.

#2 - Update formatting to be consistent. ++++++++++++++++++++++++++++++ rpmsg_get_tx_payload_buffer() The Doxygen formatting is not proper for the input parameters and therefore does not display correctly.

html output for input parameters is as follows:

@ept: Pointer to rpmsg endpoint @len: Pointer to store tx buffer size @wait: Boolean, wait or not for buffer to become available

This should instead be formatted as:

Parameters

ept    Pointer to rpmsg endpoint
len    Pointer to store tx buffer size
wait   Boolean, wait or not for buffer to become available

The changes to support this requirement are as follows:

Current formatting:

/**

• @brief Gets the tx buffer for message payload.
• This API can only be called at process context to get the tx buffer in vring.
• By this way, the application can directly put its message into the vring tx
• buffer without copy from an application buffer.
• It is the application responsibility to correctly fill the allocated tx
• buffer by data and passing correct parameters to the

rpmsg_send_nocopy() or

• rpmsg_sendto_nocopy() function to perform data no-copy-send

mechanism.

• @ept: Pointer to rpmsg endpoint
• @len: Pointer to store tx buffer size
• @wait: Boolean, wait or not for buffer to become available
• @return The tx buffer address on success and NULL on failure
• @see rpmsg_send_offchannel_nocopy
• @see rpmsg_sendto_nocopy
• @see rpmsg_send_nocopy

*/ void *rpmsg_get_tx_payload_buffer(struct rpmsg_endpoint *ept, uint32_t *len, int wait);

Proposed updated formatting highlighted by ==>:

/**

• @brief Gets the tx buffer for message payload.
• This API can only be called at process context to get the tx buffer in vring.
• By this way, the application can directly put its message into the vring tx
• buffer without copy from an application buffer.
• It is the application responsibility to correctly fill the allocated tx
• buffer by data and passing correct parameters to the

rpmsg_send_nocopy() or

• rpmsg_sendto_nocopy() function to perform data no-copy-send

mechanism.

==> * @param ept Pointer to rpmsg endpoint ==> * @param len Pointer to store tx buffer size ==> * @param wait Boolean, wait or not for buffer to become available

Is the syntax support ":"? Would be nice to also maximize the readability in the source code * @param ept: Pointer to rpmsg endpoint * @param len: Pointer to store tx buffer size * @param wait: Boolean, wait or not for buffer to become available * @param ept : Pointer to rpmsg endpoint

If not possible we can use tabs to align descriptions * @param ept Pointer to rpmsg endpoint * @param len Pointer to store tx buffer size * @param wait Boolean, wait or not for buffer to become available * @param ept Pointer to rpmsg endpoint

• @return The tx buffer address on success and NULL on failure
• @see rpmsg_send_offchannel_nocopy
• @see rpmsg_sendto_nocopy
• @see rpmsg_send_nocopy

*/ void *rpmsg_get_tx_payload_buffer(struct rpmsg_endpoint *ept, uint32_t *len, int wait);

#3 - Ensure all parameters are documented. ++++++++++++++++++++++++++++++++++ rpmsg_send_offchannel_raw() The Doxygen comments in the source file are complete, but the header file is missing some parameters. All routines should be checked for completeness.

Does a tool exists for that? If yes should be quite simple to add a github action to check a Pull request.

Thanks, Arnaud

Comments respectfully appreciated.

Thanks, Tammy

-- Openamp-system-reference mailing list -- openamp-system- reference@lists.openampproject.org To unsubscribe send an email to openamp-system-reference- leave@lists.openampproject.org