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

wolfHSM Manual (migration from wolfHSM-docs to wolfssl/documentation) #131

Merged
merged 8 commits into from
Jun 6, 2024

Conversation

aidangarske
Copy link

@aidangarske aidangarske commented Jun 5, 2024

wolfHSM documentation generation.

The wolfHSM does not yet have API documentation, so the Doxygen steps are being skipped. Only the src/*.md files are being used for the HTML and PDF manuals.

Updated the mkdocs.yml and took out the SUMMARY.md because it is not necessary in documentations repo.

Adds new V=1 option to help with showing commands being used. Updated copyright.

@dgarske
Copy link
Contributor

dgarske commented Jun 5, 2024

The failure is due to the existing #17 1.500 cat: api/md/tpm2_8h.md: No such file or directory issue. I'm going to try and fix that right now.

I locally built make wolfhsm and got the error:

1.661 pandoc: SUMMARY.md: withBinaryFile: does not exist (No such file or directory)

wolfHSM-Manual.pdf

Fix pushed. Attached is the wolfHSM User Manual for review.

@dgarske dgarske self-assigned this Jun 5, 2024
@dgarske dgarske changed the title wolfHSM-docs to wolfSSL-documentation integration - wolfHSM Manual wolfHSM Manual (migration from wolfHSM-docs to wolfssl/documentation) Jun 5, 2024
Copy link
Contributor

@bigbrett bigbrett left a comment

Choose a reason for hiding this comment

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

Awesome! First quick pass flagging some broken links, spelling errors, and a few suggestions.

wolfHSM/src/chapter03.md Outdated Show resolved Hide resolved
wolfHSM/src/chapter03.md Outdated Show resolved Hide resolved
wolfHSM/src/chapter03.md Outdated Show resolved Hide resolved
wolfHSM/src/chapter04.md Outdated Show resolved Hide resolved
wolfHSM/src/chapter04.md Outdated Show resolved Hide resolved
wolfHSM/src/chapter05.md Outdated Show resolved Hide resolved
wolfHSM/src/chapter05.md Outdated Show resolved Hide resolved

When using wolfCrypt in the client application, compatible crypto operations can be executed on the wolfHSM server by passing `WOLFHSM_DEV_ID` as the `devId` argument. The wolfHSM client must be initialized before using any wolfHSM remote crypto.

If wolfHSM does not yet support that algorithm, the API call will return `CRYPTOCB_UNAVAILABLE`. See [supported wolfCrypt algorithms](todo) for the full list of algorithms wolfHSM supports for remote HSM execution.
Copy link
Contributor

Choose a reason for hiding this comment

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

broken link, but this section doesn't exist. I can add it in a PR later

wolfHSM/src/chapter07.md Outdated Show resolved Hide resolved
wolfHSM/src/chapter08.md Outdated Show resolved Hide resolved
@bigbrett
Copy link
Contributor

bigbrett commented Jun 5, 2024

@aidangarske You mentioned that we don't yet have doxygen - we do have the client and server APIs doxygen-commented, but it is inline in the source code (as opposed to maintained separately like wolfSSL does in this repo). There isnt a doxyfile yet though. Just FYI on where that is at

@dgarske
Copy link
Contributor

dgarske commented Jun 5, 2024

@aidangarske You mentioned that we don't yet have doxygen - we do have the client and server APIs doxygen-commented, but it is inline in the source code (as opposed to maintained separately like wolfSSL does in this repo). There isnt a doxyfile yet though. Just FYI on where that is at

@aidangarske is OOO today, but @rlm2002 can help. Brett if you have API's that are public that you would like Doxygen to process please provide a link to those. Aidan and I could not find them.

/*!
    \ingroup Client Functions
    \brief Description of function

    \return 0=

    \param arg1 description 

    _Example_
    \code
    int rc = CallFunc();
    \endcode

    \sa OtherFunction
*/

Thanks

@bigbrett
Copy link
Contributor

bigbrett commented Jun 5, 2024

@aidangarske You mentioned that we don't yet have doxygen - we do have the client and server APIs doxygen-commented, but it is inline in the source code (as opposed to maintained separately like wolfSSL does in this repo). There isnt a doxyfile yet though. Just FYI on where that is at

@aidangarske is OOO today, but @rlm2002 can help. Brett if you have API's that are public that you would like Doxygen to process please provide a link to those. Aidan and I could not find them.

@rlm2002 here ya go. We haven't set up the client/server groups yet

aidan garske and others added 8 commits June 5, 2024 15:42
…yet have API documentation, so the Doxygen steps are being skipped. Only the src/*.md files are being used for the HTML and PDF manuals. updated the mkdocs.yml and took out the summary because it is not necessary in documentations repo.
…to see actual commands. Removed tracking of generated wolfHSM/docs/xml files.
@JacobBarthelmeh
Copy link
Contributor

Changes look good to me as a nice first step to adding the wolfHSM manual. The github actions is complaining about:

> [wolfssl-stage1 2/2] RUN make pdf V=:
0.992 [info] Loading docs/xml/wh__flash_8h.xml
0.992 [info] Loading docs/xml/wh__error_8h.xml
Warning: 0.993 [warning] Failed to parse member wh__error_8h error: basic_string: construction from null is not valid
0.993 [info] Loading docs/xml/wh__common_8h.xml
Warning: 0.994 [warning] Failed to parse member wh__common_8h error: basic_string: construction from null is not valid
0.994 [info] Loading docs/xml/README_8md.xml

@dgarske
Copy link
Contributor

dgarske commented Jun 6, 2024

Changes look good to me as a nice first step to adding the wolfHSM manual. The github actions is complaining about:

> [wolfssl-stage1 2/2] RUN make pdf V=:
0.992 [info] Loading docs/xml/wh__flash_8h.xml
0.992 [info] Loading docs/xml/wh__error_8h.xml
Warning: 0.993 [warning] Failed to parse member wh__error_8h error: basic_string: construction from null is not valid
0.993 [info] Loading docs/xml/wh__common_8h.xml
Warning: 0.994 [warning] Failed to parse member wh__common_8h error: basic_string: construction from null is not valid
0.994 [info] Loading docs/xml/README_8md.xml

Those are fixed with wolfSSL/wolfHSM#42
Caused by unnamed enums and unions.
Once that PR is merged this should work. I've tested locally.

@JacobBarthelmeh
Copy link
Contributor

JacobBarthelmeh commented Jun 6, 2024

@bigbrett is the request for changes still outstanding? Am okay merging in this initial documentation addition as is.

@bigbrett
Copy link
Contributor

bigbrett commented Jun 6, 2024

@bigbrett is the request for changes still outstanding? Am okay merging in this initial documentation addition as is.

@JacobBarthelmeh let me take a peek. I'll approve if they are all addressed, otherwise I'll request changes.

Copy link
Contributor

@bigbrett bigbrett left a comment

Choose a reason for hiding this comment

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

@JacobBarthelmeh good to go.

@dgarske @aidangarske thank you both so much for your help on this. Go Team Garske!!!

@dgarske @JacobBarthelmeh Regarding the doxybook2 error, naming the enums is fine, but naming the unions will require changes to the calling code to work. We might end up doing that, or we may just pull docs into a separate header like wolfSSL does. Will discuss with the team. I don't think anonymous unions are critical to funcitonality, so we might just bite the bullet and refactor in this case. I'll keep you in the loop, but go ahead and merge for now without the API docs live.

@JacobBarthelmeh JacobBarthelmeh merged commit 7570a07 into wolfSSL:master Jun 6, 2024
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants