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

Added a warning about RF24 preprocessor flag being deprecated #1159

Closed
wants to merge 3 commits into from
Closed

Conversation

Avamander
Copy link
Contributor

Fixes #1123

@@ -270,7 +270,7 @@
*/
// legacy
#ifdef MY_RADIO_NRF24
//MY_RADIO_NRF24 is deprecated
#warning MY_RADIO_NRF24 is deprecated, use MY_RADIO_RF24 instead.
Copy link
Contributor

@fallberg fallberg Jul 6, 2018

Choose a reason for hiding this comment

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

In addition to a compiler warning when defined, I think it might also be a good idea to add a doxygen block marking the define deprecated (see https://www.stack.nl/~dimitri/doxygen/manual/commands.html#cmddeprecated)

Something along the lines of

 /**
  * @def MY_RADIO_NRF24
  * @brief Define this to use a RF24-based radio transport for sensor network communication.
  * @deprecated This flag is deprecated and replaced by @ref MY_RADIO_RF24
  */
 //#define MY_RADIO_RF24

To allow doxygen to pick up the define, it also need to be defined below under the doxygen specific segment (see a7299d9#diff-cf14b6301beb3a4dbaded9b95bb190feL2295)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, okay, but where exactly do I add it?

Copy link
Contributor

Choose a reason for hiding this comment

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

At my comment

Copy link
Contributor

Choose a reason for hiding this comment

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

If you don't have doxygen locally, you can examine the results from the build server by clicking "Details" next to the "Toll gate (Documentation)" check below.

Copy link
Member

Choose a reason for hiding this comment

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

@Avamander is there anything we can do to assist you?

Copy link
Contributor

Choose a reason for hiding this comment

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

@Avamander Please submit this PR against mysensors:development, not mysensors:development-3.0.0

@henrikekblad
Copy link
Member

Closing this PR targeted against the wrong branch. @Avamander, please open it again against the development-branch with the comments above fixed.

@tekka007 tekka007 mentioned this pull request Aug 21, 2018
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