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

address some Doxygen warnings and issue #632 #639

Merged
merged 19 commits into from
Sep 24, 2020
Merged

Conversation

2bndy5
Copy link
Member

@2bndy5 2bndy5 commented Sep 24, 2020

this PR addresses issue #632 & some doxygen warnings caused by

  • differences in function signatures and their corresponding documented parameters
  • doxygen trying to automatically reference syntax used in examples

Also I've updated the Doxygen file's PROJECT_NUMBER tag used to display what version the docs are built for. It now reads V1.3.9

These are the warnings that have been addressed:

C:/Users/ytreh/Documents/Github/RF24/RF24.h:702: warning: unbalanced grouping commands
. . .
C:/Users/ytreh/Documents/Github/RF24/utility/Template/compatibility.h:48: warning: end of file while inside a group
. . .
C:/Users/ytreh/Documents/Github/RF24/utility/Template/gpio.h:64: warning: end of file while inside a group
. . .
C:/Users/ytreh/Documents/Github/RF24/utility/Template/includes.h:31: warning: end of file while inside a group
. . .
C:/Users/ytreh/Documents/Github/RF24/utility/Template/RF24_arch_config.h:84: warning: end of file while inside a group
. . .
C:/Users/ytreh/Documents/Github/RF24/utility/Template/spi.h:82: warning: end of file while inside a group
. . .
C:/Users/ytreh/Documents/Github/RF24/RF24.h:1560: warning: explicit link request to 'define' could not be resolved
. . .
C:/Users/ytreh/Documents/Github/RF24/RF24.h:1948: warning: Unsupported xml/html tag <version> found
. . .
C:/Users/ytreh/Documents/Github/RF24/utility/Template/gpio.h:45: warning: argument 'value' of command @param is not found in the argument list of read(int port)
. . .
C:/Users/ytreh/Documents/Github/RF24/RF24.h:122: warning: explicit link request to 'define' could not be resolved
C:/Users/ytreh/Documents/Github/RF24/RF24.h:115: warning: argument 'spispeed' of command @param is not found in the argument list of RF24::RF24(uint16_t _cepin, uint16_t _cspin, uint32_t _spispeed=RF24_SPI_SPEED)
C:/Users/ytreh/Documents/Github/RF24/RF24.h:132: warning: The following parameter of RF24::RF24(uint16_t _cepin, uint16_t _cspin, uint32_t _spispeed=RF24_SPI_SPEED) is not documented:
  parameter '_spispeed'
C:/Users/ytreh/Documents/Github/RF24/RF24.h:502: warning: The following parameter of RF24::txStandBy(uint32_t timeout, bool startTx=0) is not documented:
  parameter 'startTx'
C:/Users/ytreh/Documents/Github/RF24/RF24.h:570: warning: The following parameter of RF24::startFastWrite(const void *buf, uint8_t len, const bool multicast, bool startTx=1) is not documented:
  parameter 'startTx'

This is the only warning that I have not been able to addresses: (seems to be a doxygen problem)

C:/Users/ytreh/Documents/Github/RF24/Makefile:128: warning: Reached end of file while still inside a (nested) comment. Nesting level 5 (probable line reference: 125, 124, 123, 112, 111)

PROGMEM placement

Doxygen gets confused because the keyword PROGMEM was placed after the variable name in its declaration. Here is a quote from arduino.cc about PROGMEM placement:

... experiments have indicated that, in various versions of Arduino (having to do with GCC version), PROGMEM may work in one location and not in another. The "string table" example below has been tested to work with Arduino 13. Earlier versions of the IDE may work better if PROGMEM is included after the variable name.

Query

If we want to continue support for Arduino 13 (released in Feb 2009) or prior, I can undo the PROGMEM placement changes. But if not we should specify that the latest Arduino IDE version 14 or later is required.

With all that said, changing
static const <datatype> var_name[] PROGMEM = { ... };
to
static const PROGMEM <datatype> var_name[] = { ... };
fixes the following warnings.

C:/Users/ytreh/Documents/Github/RF24/RF24.cpp:476: warning: documented symbol 'static const char rf24_datarate_e_str_1' was not declared or defined.
C:/Users/ytreh/Documents/Github/RF24/RF24.cpp:477: warning: documented symbol 'static const char rf24_datarate_e_str_2' was not declared or defined.
C:/Users/ytreh/Documents/Github/RF24/RF24.cpp:478: warning: documented symbol 'static const char *const rf24_datarate_e_str_P' was not declared or defined.
C:/Users/ytreh/Documents/Github/RF24/RF24.cpp:483: warning: documented symbol 'static const char rf24_model_e_str_0' was not declared or defined.
C:/Users/ytreh/Documents/Github/RF24/RF24.cpp:484: warning: documented symbol 'static const char rf24_model_e_str_1' was not declared or defined.
C:/Users/ytreh/Documents/Github/RF24/RF24.cpp:485: warning: documented symbol 'static const char *const rf24_model_e_str_P' was not declared or defined.
C:/Users/ytreh/Documents/Github/RF24/RF24.cpp:489: warning: documented symbol 'static const char rf24_crclength_e_str_0' was not declared or defined.
C:/Users/ytreh/Documents/Github/RF24/RF24.cpp:490: warning: documented symbol 'static const char rf24_crclength_e_str_1' was not declared or defined.
C:/Users/ytreh/Documents/Github/RF24/RF24.cpp:491: warning: documented symbol 'static const char rf24_crclength_e_str_2' was not declared or defined.
C:/Users/ytreh/Documents/Github/RF24/RF24.cpp:492: warning: documented symbol 'static const char *const rf24_crclength_e_str_P' was not declared or defined.
C:/Users/ytreh/Documents/Github/RF24/RF24.cpp:497: warning: documented symbol 'static const char rf24_pa_dbm_e_str_0' was not declared or defined.
C:/Users/ytreh/Documents/Github/RF24/RF24.cpp:498: warning: documented symbol 'static const char rf24_pa_dbm_e_str_1' was not declared or defined.
C:/Users/ytreh/Documents/Github/RF24/RF24.cpp:499: warning: documented symbol 'static const char rf24_pa_dbm_e_str_2' was not declared or defined.
C:/Users/ytreh/Documents/Github/RF24/RF24.cpp:500: warning: documented symbol 'static const char rf24_pa_dbm_e_str_3' was not declared or defined.
C:/Users/ytreh/Documents/Github/RF24/RF24.cpp:501: warning: documented symbol 'static const char *const rf24_pa_dbm_e_str_P' was not declared or defined.
C:/Users/ytreh/Documents/Github/RF24/RF24.cpp:717: warning: documented symbol 'static const uint8_t child_pipe_enable' was not declared or defined.
C:/Users/ytreh/Documents/Github/RF24/RF24.cpp:1150: warning: documented symbol 'static const uint8_t child_pipe' was not declared or defined.
C:/Users/ytreh/Documents/Github/RF24/RF24.cpp:1152: warning: documented symbol 'static const uint8_t child_payload_size' was not declared or defined.

@2bndy5
Copy link
Member Author

2bndy5 commented Sep 24, 2020

Did some testing with an unrelated C++ repo of mine about using breathe sphinx extension that takes doxygen XML output and generates Sphinx documentation (with additional rST files). I do not recommend it! While it works there are some visual disadvangtages compared to normal rST usage in Sphinx. I also tried the sister project called exhale that depends on breathe, and the only difference is that exhale automatically runs doxygen for you (using some customized doxygen file tags via CLI args) which allow for a little more rST flexibility. But exhale ultimately requires you to abandon doxygen's HTML output altogether (due to the customized CLI args).

That lead about m.css looks ok. However, their doxygen theme seems to require other dependencies to getting it working. ☹️

I've decided to try using doxygen HTML output with a custom doxygen XML layout file, so that I could incorporate the Bulma CSS framework (true drop-in-place functionality, mobile friendly, & no JS needed). I realize it requires some knowledge of CSS (specifically the class property of any HTML tags that are to be modified), but its a set-and-forget solution to updating that "Netscape" look that doxygen sports.

@2bndy5 2bndy5 linked an issue Sep 24, 2020 that may be closed by this pull request
@Avamander
Copy link
Member

seems to require other dependencies to getting it working.

How's that an issue?

@Avamander Avamander merged commit f2185d6 into master Sep 24, 2020
@Avamander Avamander deleted the doxygen-warnings branch September 24, 2020 12:10
@2bndy5
Copy link
Member Author

2bndy5 commented Sep 24, 2020

Its not a huge issue, but after trying the breathe/exhale solutions for rtd.org, I was hoping for a solution in which the dependencies are provided with the repo. Primarily, I don't want to impose more dependencies on those who might want to contribute to the docs as local testing is highly recommended in that situation.

So initial tests with Bulma CSS didn't work out well. Turns out the div elements stylizing the navbar are generated completely, and injecting property names like class would require JS (which kinda defeats the purpose of using a no JS framework) -- not to mention all animation from the navbar is done via generated JS (could be done with CSS though). I'll give m.css a try, but I generally try to avoid jinja. Its like python but for HTML & JSON (only works well for string values -- meaning numbers are actually strings).

@2bndy5
Copy link
Member Author

2bndy5 commented Sep 24, 2020

should I raise an issue about the Arduino IDE requirements as this PR changed the minimum version slightly, or are you not worried?

@Avamander
Copy link
Member

should I raise an issue about the Arduino IDE requirements as this PR changed the minimum version slightly, or are you not worried?

I'm not.

Primarily, I don't want to impose more dependencies on those who might want to contribute to the docs as local testing is highly recommended in that situation.

Building the final HTML docs is not super mandatory when adding a bit of new documentation or changing existing one, so it really isn't imposing it on everyone.

@2bndy5
Copy link
Member Author

2bndy5 commented Sep 24, 2020

🤔 I thought the docs are updated only when a contributor builds the docs and then uploads the changes to TMRh20's repo (which is then approved by another PR to get them live via github pages). Notice that this PR hasn't actually affected the github pages deployment... yet.

@Avamander
Copy link
Member

Yes, exactly, they're built by a contributor that uploads them, not everyone who makes a PR.

@2bndy5
Copy link
Member Author

2bndy5 commented Sep 24, 2020

could be automated better via github actions, but that's an issue for another time because the docs (in TMRh20's repo) are spread out among the RF24* repos. I have a simple example of this.

@Avamander
Copy link
Member

Theoretically the repository can be configured to serve a subfolder of the repository under the github.io domain, but that needs setting up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

docs on read() need additional information
2 participants