Skip to content

drivers/mrf24j40 add tx retries needed get operation#7448

Merged
smlng merged 1 commit intoRIOT-OS:masterfrom
bergzand:mrf24j40-tx-retries
Sep 28, 2017
Merged

drivers/mrf24j40 add tx retries needed get operation#7448
smlng merged 1 commit intoRIOT-OS:masterfrom
bergzand:mrf24j40-tx-retries

Conversation

@bergzand
Copy link
Copy Markdown
Member

@bergzand bergzand commented Aug 6, 2017

requires #7447

@bergzand
Copy link
Copy Markdown
Member Author

Rebased because #7447 was merged. No longer depends on another PR.

@jnohlgard jnohlgard added Area: drivers Area: Device drivers Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation Area: network Area: Networking labels Aug 17, 2017
@jnohlgard jnohlgard added this to the Release 2017.10 milestone Aug 17, 2017
@bergzand
Copy link
Copy Markdown
Member Author

Rebased to resolve merge conflicts

Copy link
Copy Markdown
Member

@smlng smlng left a comment

Choose a reason for hiding this comment

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

some minor nits

/**
* @name Shift offsets for TXSTAT register (0x24)
* @{
*/
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

please put in parentheses and make explicitly unsigned, i.e (6U) - might not be necessary but would be more consistent with (majority) of RIOT code base.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Ack, fixed

* @name Shift offsets for TXSTAT register (0x24)
* @{
*/
#define MRF24J40_TXSTAT_MAX_FRAME_RETRIES_SHIFT 6
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

not used (yet?)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Correct, I added it for completeness of that register definition

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

fine by me, just asking.

#ifdef MODULE_NETSTATS_L2
if (netdev->event_callback && (dev->netdev.flags & MRF24J40_OPT_TELL_TX_END)) {
uint8_t txstat = mrf24j40_reg_read_short(dev, MRF24J40_REG_TXSTAT);
dev->tx_retries = (txstat >> MRF24J40_TXSTAT_MAX_FRAME_RETRIES_SHIFT);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

looks like a tab here? Change to spaces.

@smlng smlng added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Sep 26, 2017
@bergzand
Copy link
Copy Markdown
Member Author

@smlng Thank you for you comments, I think I've addressed them all.

@smlng
Copy link
Copy Markdown
Member

smlng commented Sep 26, 2017

ACK, please squash

@bergzand
Copy link
Copy Markdown
Member Author

squashed

@smlng
Copy link
Copy Markdown
Member

smlng commented Sep 28, 2017

Time to move this forward: ACK and GO!

@smlng smlng merged commit 671cb9c into RIOT-OS:master Sep 28, 2017
@bergzand bergzand deleted the mrf24j40-tx-retries branch September 29, 2017 09:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: drivers Area: Device drivers Area: network Area: Networking 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.

5 participants