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

drivers/at86rf231: refactoring of the at86rf231 radio driver #1846

Merged
merged 2 commits into from
Oct 31, 2014
Merged

drivers/at86rf231: refactoring of the at86rf231 radio driver #1846

merged 2 commits into from
Oct 31, 2014

Conversation

thomaseichinger
Copy link
Member

  • deploy extended operation mode
  • cleanup
  • implement netdev 802154.h interface

@thomaseichinger thomaseichinger added Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Area: network Area: Networking labels Oct 20, 2014
@thomaseichinger thomaseichinger added this to the Release NEXT MAJOR milestone Oct 20, 2014
@thomaseichinger
Copy link
Member Author

depends on #1830

Kijewski
Kijewski reviewed Oct 20, 2014
View reviewed changes
drivers/at86rf231/at86rf231.c
at86rf231_reg_write(AT86RF231_REG__TRX_STATE, AT86RF231_TRX_STATE__FORCE_TRX_OFF);

/* busy wait for TRX_OFF state */
uint8_t delay = 100;
Copy link
Contributor

Choose a reason for hiding this comment

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

Please declare this variable as an int in both do … while blocks.

@Kijewski
Copy link
Contributor

Please do a null-edit, Travis did not run for your PR.

@thomaseichinger
Copy link
Member Author

addressed @Kijewski's comments

@thomaseichinger thomaseichinger added the CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable label Oct 21, 2014
@thomaseichinger thomaseichinger force-pushed the at86rf23x branch 2 times, most recently from 02fed6d to 181b1fd Compare October 21, 2014 12:58
@OlegHahm
Copy link
Member

txtsnd in default example is not working any more.

@thomaseichinger thomaseichinger added the State: waiting for other PR State: The PR requires another PR to be merged first label Oct 22, 2014
@thomaseichinger
Copy link
Member Author

@OlegHahm did you pull in #1830?

@thomaseichinger
Copy link
Member Author

rebase on master

@OlegHahm
Copy link
Member

No, didn't saw the comment and label was missing. Will test again.

@OlegHahm
Copy link
Member

Still not working.

@OlegHahm
Copy link
Member

Can you set the default PAN id to 1 in driver init (if module config is not used) and extend the txtsend command by a third parameter specifying an optional PAN id?

@thomaseichinger
Copy link
Member Author

addressed @OlegHahm's comment

@OlegHahm
Copy link
Member

Works with default example.

OlegHahm
OlegHahm reviewed Oct 30, 2014
View reviewed changes
drivers/include/at86rf231.h
#define AT86RF231_BROADCAST_ADDRESS (0xFFFF)

/**
* @biref at86rf231's lowest supported channel
Copy link
Member

Choose a reason for hiding this comment

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

typo

@OlegHahm
Copy link
Member

I know it's a little bit out of scope, but since these are the last three doxygen warnings for the driver, do you mind to fix them, too?

RIOT/drivers/include/at86rf231.h:10: warning: missing title after \defgroup drivers_at86rf231
RIOT/drivers/include/at86rf231.h:87: warning: Member AT_DRIVER_STATE_DEFAULT (macro definition) of group drivers_at86rf231 is not documented.
RIOT/drivers/include/at86rf231.h:88: warning: Member AT_DRIVER_STATE_SENDING (macro definition) of group drivers_at86rf231 is not documented.

OlegHahm
OlegHahm reviewed Oct 30, 2014
View reviewed changes
drivers/include/netdev/802154.h
/**
* @see netdev_driver_t::get_option
*
* @detail The options are constrained as follows:
Copy link
Member

Choose a reason for hiding this comment

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

@details

@thomaseichinger
Copy link
Member Author

addressed @OlegHahm's comments and rebased.

@thomaseichinger thomaseichinger added the CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable label Oct 30, 2014
@OlegHahm
Copy link
Member

Squash and go.

@thomaseichinger thomaseichinger force-pushed the at86rf23x branch 3 times, most recently from 02869ed to bb82d9e Compare October 30, 2014 19:03
@OlegHahm OlegHahm removed the CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable label Oct 30, 2014
@thomaseichinger thomaseichinger force-pushed the at86rf23x branch 2 times, most recently from cb0ae1c to 9e2cfdc Compare October 30, 2014 21:34
@thomaseichinger
drivers/at86rf231: refactoring of the at86rf231 radio driver
dfb1b56 
* deploy extended operation mode
* cleanup
* implement netdev 802154.h interface
@thomaseichinger
netdev/802154: suppress cppcheck warnings in 802154.h
9e2cfdc 
Suppress cppcheck warnings of class unusedStructMember because
this interface is not used yet. Remove these comments once it
is used.
OlegHahm added a commit that referenced this pull request Oct 31, 2014
@OlegHahm
Merge pull request #1846 from thomaseichinger/at86rf23x
fce3a22 
drivers/at86rf231: refactoring of the at86rf231 radio driver
@OlegHahm OlegHahm merged commit fce3a22 into RIOT-OS:master Oct 31, 2014
@thomaseichinger
Copy link
Member Author

\o/

@OlegHahm OlegHahm mentioned this pull request Oct 31, 2014
Merged
N8Fear
N8Fear reviewed Oct 31, 2014
View reviewed changes
drivers/at86rf231/at86rf231.c
}
/* read CSMA_SEED_1 and write back with RNG value */
uint8_t tmp = at86rf231_reg_read(AT86RF231_REG__CSMA_SEED_1);
tmp = ((at86rf231_reg_read(AT86RF231_REG__PHY_CC_CCA)&0x60)>>5);
Copy link

Choose a reason for hiding this comment

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

I get a redundant assignment cppcheck warning for tmp here that seems to make sense.

@N8Fear
Copy link

N8Fear commented Oct 31, 2014

@thomaseichinger Could you please adress the two comments I made in a quick followup PR?

@thomaseichinger
Copy link
Member Author

@N8Fear good catches, see #1920.

This was referenced Nov 18, 2014
Closed
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@OlegHahm OlegHahm

Labels
Area: network Area: Networking Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation
Projects
None yet
Milestone
Release 2014.12
Development

Successfully merging this pull request may close these issues.
4 participants
@thomaseichinger @Kijewski @OlegHahm @N8Fear