Skip to content

at86rf2xx: Always set channel on device#9616

Merged
PeterKietzmann merged 1 commit intoRIOT-OS:masterfrom
bergzand:pr/at86rf2xx/always_set_channel
Jul 31, 2018
Merged

at86rf2xx: Always set channel on device#9616
PeterKietzmann merged 1 commit intoRIOT-OS:masterfrom
bergzand:pr/at86rf2xx/always_set_channel

Conversation

@bergzand
Copy link
Copy Markdown
Member

@bergzand bergzand commented Jul 20, 2018

Contribution description

This removes the check if the current configured channel equals the new
channel. This check prevents the at86rf2xx channel to be configured
after a reset which causes the radio to be non-functional after a
NETOPT_STATE_RESET.

Issues/PRs references

fixes #8118

Testing

Branch is intentionally not based on the latest master to prevent #9577/#9606 from causing issues.

The issue that causes gnrc_netif to switch to short source MAC addresses is not resolved here.

@bergzand
at86rf2xx: Always set channel on device
b025362 
This removes the check if the current configured channel equals the new
channel. This check prevents the at86rf2xx channel to be configured
after a reset which causes the radio to be non-functional after a
NETOPT_STATE_RESET.
@bergzand bergzand added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Area: network Area: Networking Area: drivers Area: Device drivers labels Jul 20, 2018
@bergzand bergzand requested review from PeterKietzmann and smlng July 20, 2018 10:21
@bergzand bergzand added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Process: needs backport Integration Process: The PR is required to be backported to a release or feature branch labels Jul 20, 2018
smlng
smlng approved these changes Jul 21, 2018
View reviewed changes
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.

untested ACK (no hardware right now), but looks good.

@PeterKietzmann
Copy link
Copy Markdown
Member

PeterKietzmann commented Jul 23, 2018 •
edited
Loading

This PR fixes the described issue after resetting the interface for the first time (ifconfig 7 set state reset). However, after resetting it a second time I can't ping anymore. Doing a button reset, everything works fine again. I did not further investigate yet. Ideas?

@PeterKietzmann
Copy link
Copy Markdown
Member

PeterKietzmann commented Jul 23, 2018

This is the ping6 request after the second netif reset command (ifconfig 7 set state reset). It seems that incoming packets are not received anymore.

sniffy

@bergzand
Copy link
Copy Markdown
Member Author

bergzand commented Jul 23, 2018

I'd have to investigate a bit more. For the moment I think we can conclude that this is only a partial fix.

@smlng
Copy link
Copy Markdown
Member

smlng commented Jul 31, 2018 •
edited
Loading

I can confirm the issue. Tested with samr21-xpro and examples/gnrc_networking, after two ifconfig 7 set state reset the board does not receive anymore - but it can send, i.e., I see pings from the board to another node and even the replies by the other node in the sniffer, but the samr21 still reports timeouts for the pings.

Fun fact: I tested on master [too, and the issue is there as well], so this PR is not the problem.

@smlng
Copy link
Copy Markdown
Member

smlng commented Jul 31, 2018

hence, my ACK holds - now a tested one. @PeterKietzmann you wanna file an issue, as you found it.

@bergzand
Copy link
Copy Markdown
Member Author

bergzand commented Jul 31, 2018

I suspect that the issue fixed here is only one of the issues that causes problems after a reset. (One of) the other issues is that the flag set by NIB to use long addresses is cleared by the reset and not reapplied.

@smlng
Copy link
Copy Markdown
Member

smlng commented Jul 31, 2018

interesting: after two resets the node does not respond to direct pings anymore but it does to pings on ff02::1 aka multicast. I first thought it might be something messed up with the RX state or the respective interrupts, but when multicast works this cannot be the case. So it seems that the MAC address matching does is messed up?

@smlng
Copy link
Copy Markdown
Member

smlng commented Jul 31, 2018

same issue on PhyNode (pba-d-01-kw2x) so its more general and also (very likely) not driver related but in a higher layer (or conceptual how radio drivers interact with higher layers).

@PeterKietzmann PeterKietzmann mentioned this pull request Jul 31, 2018
Closed
@PeterKietzmann
Copy link
Copy Markdown
Member

PeterKietzmann commented Jul 31, 2018

See #9656 and extend, if needed.

PeterKietzmann
PeterKietzmann approved these changes Jul 31, 2018
View reviewed changes
Copy link
Copy Markdown
Member

@PeterKietzmann PeterKietzmann left a comment

Choose a reason for hiding this comment

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

ACK

@PeterKietzmann PeterKietzmann merged commit e897826 into RIOT-OS:master Jul 31, 2018
@miri64 miri64 mentioned this pull request Jul 31, 2018
Closed
@cladmi
Copy link
Copy Markdown
Contributor

cladmi commented Aug 11, 2018

When migrating the last Pull Requests for the release, I noticed this backporting-needed PR was not backported to the release.

Due to the tests being now all run I would find it hard to backport it now.
I will note it as a known issue.

This also show that this should be integrated to one of the automated tests.

@cladmi cladmi added this to the Release 2018.10 milestone Aug 11, 2018
cladmi added a commit to cladmi/RIOT that referenced this pull request Aug 11, 2018
@cladmi
Add infos on at86rf2xx issue with channel reset
9ae2836 
RIOT-OS#9616

Issue was at the end not backported
@cladmi cladmi removed the Process: needs backport Integration Process: The PR is required to be backported to a release or feature branch label Aug 11, 2018
cladmi added a commit to cladmi/RIOT that referenced this pull request Aug 13, 2018
@cladmi
Add infos on at86rf2xx issue with channel reset
c94901b 
RIOT-OS#9616

Issue was at the end not backported
cladmi added a commit to cladmi/RIOT that referenced this pull request Aug 13, 2018
@cladmi
Add infos on at86rf2xx issue with channel reset
d24314a 
RIOT-OS#9616

Issue was at the end not backported
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@PeterKietzmann PeterKietzmann PeterKietzmann approved these changes

+1 more reviewer

@smlng smlng smlng approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

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: bug The issue reports a bug / The PR fixes a bug (including spelling errors)

Projects

None yet

Milestone

Release 2018.10

Development

Successfully merging this pull request may close these issues.

at86rf2xx: netdev reset

4 participants

@bergzand @PeterKietzmann @smlng @cladmi