Skip to content

examples/tests: remove redundant xbee config#6982

Merged
aabadie merged 2 commits intoRIOT-OS:masterfrom
haukepetersen:fix_gnrcmin_xbeeconfig
May 3, 2017
Merged

examples/tests: remove redundant xbee config#6982
aabadie merged 2 commits intoRIOT-OS:masterfrom
haukepetersen:fix_gnrcmin_xbeeconfig

Conversation

@haukepetersen
Copy link
Copy Markdown
Contributor

Alternative to #6971

There is no need in configuring the xbee to use UART_DEV(1) in these Makefiles, as it is configured to use this UART per default -> see drivers/xbee/include/xbee_params.h.

@haukepetersen haukepetersen added Area: drivers Area: Device drivers Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation labels Apr 28, 2017
@haukepetersen haukepetersen added this to the Release 2017.07 milestone Apr 28, 2017
@haukepetersen haukepetersen requested review from aabadie and kYc0o April 28, 2017 13:45
Copy link
Copy Markdown
Contributor

@kYc0o kYc0o left a comment

Choose a reason for hiding this comment

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

ACK

@kYc0o kYc0o added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Apr 28, 2017
Copy link
Copy Markdown
Contributor

@aabadie aabadie left a comment

Choose a reason for hiding this comment

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

Just a minor comment on my side


BOARD_INSUFFICIENT_MEMORY := chronos msb-430 msb-430h nucleo32-f031

## Uncomment to support the XBee module
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe those 2 lines could be kept ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I am opposed to that, I don't think we should have comments about available hw/modules in these makefiles, so if we would keep the comment about the xbee here, why not also have comments about w5100, encxxx, etc.? IMHO these kind of comments don't belong here.

@kYc0o
Copy link
Copy Markdown
Contributor

kYc0o commented Apr 28, 2017

As said in #6971, it would be nice if there's written somewhere that USEMODULE += xbee would suffice to use an XBee shield.

Also, that the UART can be specified through:

XBEE_UART ?= "1"		
CFLAGS += -DXBEE_PARAM_UART=$(XBEE_UART)

@haukepetersen
Copy link
Copy Markdown
Contributor Author

@kYc0o see my comment above: I don't think this belongs here. But feel free to create a wiki page about the xbee that explains how to use it.

@aabadie
Copy link
Copy Markdown
Contributor

aabadie commented May 2, 2017

But feel free to create a wiki page about the xbee that explains how to use it.

I disagree with the wiki page. Why not adding this documentation in the xbee.h driver file as doxygen documentation ?

@haukepetersen
Copy link
Copy Markdown
Contributor Author

Because this kind of documentation is not specific to the xbee. I mean, we also don't put information on how to use a device in the default example to every single sensor driver that supports SAUL...

@aabadie
Copy link
Copy Markdown
Contributor

aabadie commented May 2, 2017

Thinking from a newcomer with a Xbee module point of view, his first question will be how can I use it with gnrc_networking or any other networking example. So this should be somewhere in the documentation:

To use the networking application with XBee, add USEMODULE+=xbee in the Makefile of the application.

or something similar. Putting it in the doxygen is imho better regarding code and documentation maintenance.

we also don't put information on how to use a device in the default example to every single sensor driver that supports SAUL...

Sure, but I don't see this as a good reason for not doing it here. We always have to start somewhere.

@haukepetersen
Copy link
Copy Markdown
Contributor Author

Thinking from a newcomer with a Xbee module point of view, his first question will be...

Yes, very true. But the same is true for many other boards/network devices/misc modules. So I think we should find a way to gracefully 'educate' the newcomers to understand our basic concepts, instead of giving them copy-paste instructions for every single peace of code we include. Don't get me wrong, I am not whatsoever going against elaborate documentation here, I am just thinking that we should focus on more generic parts (where applicable) instead of doing things that are not going to scale...

@aabadie
Copy link
Copy Markdown
Contributor

aabadie commented May 3, 2017

I am not whatsoever going against elaborate documentation here, I am just thinking that we should focus on more generic parts (where applicable) instead of doing things that are not going to scale

Fine with me. Regarding documentation, I would definitely prefer to find all documentation in a single place. Doxygen is made for that and today RIOT has very nice api doc style (thanks @miri64). For example, we could even imagine to move all boards description from the wiki to Doxygen.

But for now, we can just keep the comment (with USEMODULE+=xbee) in the application Makefile ? At least the information is somewhere.

@haukepetersen
Copy link
Copy Markdown
Contributor Author

haukepetersen commented May 3, 2017

But for now, we can just keep the comment (with USEMODULE+=xbee) in the application Makefile ? At least the information is somewhere.

no :-) Thats exactly what I don't want. Then the next person puts an out-commented USEMODULE += mrf24xx in there, and the next an out-commented USEMODULE += ble_whatsoever, all with a comment that if you want to use this or that, just comment in this line... And before we notice, half of our example makefiles are clocked up with these kind of comments... So I say let's not even start this!

@miri64
Copy link
Copy Markdown
Member

miri64 commented May 3, 2017

So I think we should find a way to gracefully 'educate' the newcomers to understand our basic concepts, instead of giving them copy-paste instructions for every single peace [sic] of code we include.

Well, there is this invention from the 60s we could use: Hypertext ;-P

@miri64
Copy link
Copy Markdown
Member

miri64 commented May 3, 2017

What I mean: we could write a doc page for our doxygen documentation (e.g. "How to enable pluggable devices"), list all devices we support that fall into that category there and link to there any time this is applicable (or at least in the examples most beginners go into first, like e.g. gnrc_networking)

@aabadie
Copy link
Copy Markdown
Contributor

aabadie commented May 3, 2017

What I mean: we could write a doc page for our doxygen documentation...

Makes a lot of sense +1, let's do this documentation job in follow-up PRs

@aabadie
Copy link
Copy Markdown
Contributor

aabadie commented May 3, 2017

opened #7001 to keep track of the auto_init documentation topic

@haukepetersen haukepetersen deleted the fix_gnrcmin_xbeeconfig branch May 4, 2017 07:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: drivers Area: Device drivers CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants