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.

Sign up for GitHub

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

Jump to bottom

Fix doxygen for Serial and RawSerial #8413

Merged
merged 6 commits into from
Oct 19, 2018
Merged

Fix doxygen for Serial and RawSerial #8413

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
84dad6c
Remove RawSerial protected member functions
Oct 12, 2018
20f913a
Remove Serial protected member functions and attributes
Oct 12, 2018
9a7f359
Add (Not Connected)
Oct 12, 2018
5ee5b07
Remove note about Stream/Serial deprecation
Oct 12, 2018
b31bc76
Add note about default baud rate is 9600
Oct 12, 2018
383798b
Revert "Remove note about Stream/Serial deprecation"
Oct 15, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions drivers/RawSerial.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ class RawSerial: public SerialBase, private NonCopyable<RawSerial> {

int printf(const char *format, ...);

#if !(DOXYGEN_ONLY)
Copy link
Contributor

Choose a reason for hiding this comment

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

Protected methods are part of the API, so should really be documented.

These are overrides of the methods in SerialBase, so should be documented there - then if we have inheritance turned on, we don't need further documentation here.

Copy link
Contributor Author

@yennster yennster Oct 15, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

@kjbracey-arm #8413 (comment)

protected:

/* Acquire exclusive access to this serial port
Expand All @@ -97,6 +98,7 @@ class RawSerial: public SerialBase, private NonCopyable<RawSerial> {
/* Release exclusive access to this serial port
*/
virtual void unlock(void);
#endif
};

} // namespace mbed
Expand Down
8 changes: 5 additions & 3 deletions drivers/Serial.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,10 @@ class Serial : public SerialBase, public Stream, private NonCopyable<Serial> {
* @param tx Transmit pin
* @param rx Receive pin
* @param name The name of the stream associated with this serial port (optional)
* @param baud The baud rate of the serial port (optional, defaults to MBED_CONF_PLATFORM_DEFAULT_SERIAL_BAUD_RATE)
* @param baud The baud rate of the serial port (optional, defaults to MBED_CONF_PLATFORM_DEFAULT_SERIAL_BAUD_RATE or 9600)
Copy link
Contributor

Choose a reason for hiding this comment

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

How so "or 9600"? I guess you mean platform.default_serial_baud_rate itself defaults to 9600?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kjbracey-arm Yeah, MBED_CONF_PLATFORM_DEFAULT_SERIAL_BAUD_RATE is 9600

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kjbracey-arm I wanted to add the or 9600 because with just a quick glance I don't care about the MBED_CONF_PLATFORM_DEFAULT_SERIAL_BAUD_RATE configuration parameter, I want to know the actual numerical value it defaults to

*
* @note
* Either tx or rx may be specified as NC if unused
* Either tx or rx may be specified as NC (Not Connected) if unused
*/
Serial(PinName tx, PinName rx, const char *name = NULL, int baud = MBED_CONF_PLATFORM_DEFAULT_SERIAL_BAUD_RATE);

Expand All @@ -78,7 +78,7 @@ class Serial : public SerialBase, public Stream, private NonCopyable<Serial> {
* @param baud The baud rate of the serial port
*
* @note
* Either tx or rx may be specified as NC if unused
* Either tx or rx may be specified as NC (Not Connected) if unused
*/
Serial(PinName tx, PinName rx, int baud);

Expand All @@ -99,13 +99,15 @@ class Serial : public SerialBase, public Stream, private NonCopyable<Serial> {
return SerialBase::writeable();
}

#if !(DOXYGEN_ONLY)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think _mutex should have been private here, hence excluded that way? And the overridden protected functions should have been already documented in Stream and SerialBase, so picked up by INHERIT_DOCS.

If you're globally trying to exclude protected API methods (despite them being API), then does this trick work? https://stackoverflow.com/questions/11316663/is-it-possible-to-prevent-doxygen-from-outputting-protected-members/11316802

Also, as EXTRACT_ALL is off, aren't these just omitted by being undocumented anyway?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@AnotherButler Can you help address the doxygen fanciness?

Copy link
Contributor

Choose a reason for hiding this comment

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

They are not omitted. Our published site shows the protected member functions.

This PR follows the method we know works. For further Doxygen questions, I suggest contacting @SenRamakri of the core OS team. The doxygen_options.json and doxyfile_options files live in mbed-os, and he typically knows a lot more about them than I do. @c1728p9 also recently put up a PR addressing Doxygen options and may also be able to help.

protected:
virtual int _getc();
virtual int _putc(int c);
virtual void lock();
virtual void unlock();

PlatformMutex _mutex;
#endif
};

} // namespace mbed
Expand Down