Skip to content

Correct network status callbacks with ethernet and nanostack #8817

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
Merged
merged 1 commit into from
Nov 30, 2018
Merged

Correct network status callbacks with ethernet and nanostack #8817

merged 1 commit into from
Nov 30, 2018

Conversation

KariHaapalehto
Copy link
Contributor

@KariHaapalehto KariHaapalehto commented Nov 20, 2018 •
edited
Loading

Ethernet-tasklet needs to be registered for emac link state changes.
Ethernet-tasklet will then handle ethernet cable connection/disconnection events.

Tested locally with K64F and ethernet cable

Pull request type

[X] Fix
[ ] Refactor
[ ] Target update
[ ] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change

@0xc0170
Copy link
Contributor

0xc0170 commented Nov 20, 2018

travis-ci/astyle — Local astyle testing has failed

Please review

@KariHaapalehto KariHaapalehto force-pushed the ethernet_cable_status_callback_correction branch from 42eb345 to d621c18 Compare November 20, 2018 09:57
@KariHaapalehto
Copy link
Contributor Author

astyle fix done

kjbracey
kjbracey previously requested changes Nov 20, 2018
View reviewed changes
Copy link
Contributor

@kjbracey kjbracey left a comment

Choose a reason for hiding this comment

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

Main change looks fine, but make sure this doesn't break the border router app somehow. Shouldn't do, but worth checking.

And cross-check with #8698

features/nanostack/mbed-mesh-api/source/ethernet_tasklet.c Outdated
arm_nwk_interface_down(tasklet_data_ptr->network_interface_id);
eventOS_timeout_cancel(tasklet_data_ptr->poll_network_status_timeout);
tasklet_data_ptr->poll_network_status_timeout = NULL;
memcpy(tasklet_data_ptr->ip, 0x0, 16);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you mean memset

features/nanostack/mbed-mesh-api/source/ethernet_tasklet.c Outdated
@@ -119,6 +120,25 @@ void enet_tasklet_main(arm_event_s *event)
case APPLICATION_EVENT:
if (event->event_id == APPL_EVENT_CONNECT) {
enet_tasklet_configure_and_connect_to_network();
} else if (event->event_id == APPL_BACKHAUL_INTERFACE_PHY_UP) {
// Ethernet cable has been pluged-in
Copy link
Contributor

Choose a reason for hiding this comment

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

"plugged in" (no hyphen either)

features/nanostack/mbed-mesh-api/source/ethernet_tasklet.c Outdated
@@ -283,7 +300,7 @@ void enet_tasklet_init(void)
}
}

int8_t enet_tasklet_network_init(int8_t device_id)
uint8_t enet_tasklet_network_init(int8_t device_id)
Copy link
Contributor

Choose a reason for hiding this comment

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

Correction should be the other way, I believe - it's the header that's wrong. network_interface_id is int8_t.

#8698 is already correcting this that way - this would conflict. Either don't touch, or make an identical change to it.

features/nanostack/mbed-mesh-api/source/ethernet_tasklet.c Outdated
@@ -257,21 +277,18 @@ int8_t enet_tasklet_connect(mesh_interface_cb callback, int8_t nwk_interface_id)
return 0;
}

int8_t enet_tasklet_disconnect(bool send_cb)
void enet_tasklet_disconnect()
Copy link
Contributor

Choose a reason for hiding this comment

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

Conflicts with #8698 - I think its version is correct.

@kjbracey
Copy link
Contributor

Actually, #8698 is possibly struggling to proceed. Maybe make identical changes to it, and then we can close that one.

@KariHaapalehto KariHaapalehto force-pushed the ethernet_cable_status_callback_correction branch from d621c18 to 19c98da Compare November 21, 2018 08:22
@KariHaapalehto
Correct network status callbacks with Nanostack.
26beb98 
Ethernet-tasklet needs to be registered for emac link state changes.
Ethernet-tasklet will then handle ethernet cable connection/disconnection events.
@KariHaapalehto KariHaapalehto force-pushed the ethernet_cable_status_callback_correction branch from 19c98da to 26beb98 Compare November 21, 2018 08:28
@KariHaapalehto
Copy link
Contributor Author

Checked that nanostack border router app is still working after these changes, also conficts with #8698 removed.

@cmonr cmonr dismissed kjbracey’s stale review November 21, 2018 14:22

Commit removed. Please re-review.

@0xc0170
Copy link
Contributor

0xc0170 commented Nov 24, 2018

@kjbracey-arm Please review

@bulislaw
Copy link
Member

bulislaw commented Nov 26, 2018 •
edited
Loading

All the PRs need to be engineering ready (marked as "needs: CI") by the end of the day (Austin time). Otherwise it won't make 5.11 and will need to come in the next release (5.12 for features, 5.11.1 for fixes and new platforms).

@0xc0170
Copy link
Contributor

0xc0170 commented Nov 26, 2018

Moved to 5.11.1 (this is a fix that can land in the next patch release)

@0xc0170
Copy link
Contributor

0xc0170 commented Nov 30, 2018

CI started

@mbed-ci
Copy link

mbed-ci commented Nov 30, 2018

Test run: SUCCESS

Summary: 4 of 4 test jobs passed
Build number : 1
Build artifacts
Build logs

@cmonr cmonr removed the needs: CI label Nov 30, 2018
@cmonr cmonr merged commit 92a0e48 into ARMmbed:master Nov 30, 2018
@kjbracey
Copy link
Contributor

kjbracey commented Dec 5, 2018

@KariHaapalehto Unfortunately #8698 seems to have died, so the problems that you also spotted and avoided touching are potentially going to go unfixed. Could you pick those up?

@KariHaapalehto
Copy link
Contributor Author

@kjbracey-arm Let's merge these changes in and I'll fix them at separate PR

@KariHaapalehto KariHaapalehto deleted the ethernet_cable_status_callback_correction branch December 10, 2018 08:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@0xc0170 0xc0170 0xc0170 approved these changes

@SeppoTakalo SeppoTakalo SeppoTakalo approved these changes

@kjbracey kjbracey kjbracey left review comments

Assignees
No one assigned
Labels
release-version: 5.11.1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@KariHaapalehto @0xc0170 @kjbracey @bulislaw @mbed-ci @SeppoTakalo @cmonr