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.

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

ESP8266: implements possibility to decide between non-blocking/blocking connect. #9421

Merged
merged 3 commits into from
Jan 24, 2019
Merged

ESP8266: implements possibility to decide between non-blocking/blocking connect. #9421

merged 3 commits into from
Jan 24, 2019

Conversation

VeijoPesonen
Copy link
Contributor

@VeijoPesonen VeijoPesonen commented Jan 18, 2019

Description

Checks for already existing connection when trying to set credentials for a new connection or when trying to call connect. This is done to avoid the situation where an already established connection would get disrupted.

Implements possibility to decide between non-blocking/blocking connect.

Pull request type

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

Reviewers

@SeppoTakalo
@michalpasztamobica
@marcuschangarm
@karsev
@teetak01

Copy link
Contributor

@michalpasztamobica michalpasztamobica 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 single question to the switch-case statement. Otherwise a very good and helpful change :)

@ciarmcom
Copy link
Member

@VeijoPesonen, thank you for your changes.
@SeppoTakalo @michalpasztamobica @marcuschangarm @karsev @ARMmbed/mbed-os-ipcore @ARMmbed/mbed-os-maintainers please review.

@0xc0170
Copy link
Contributor

0xc0170 commented Jan 18, 2019

Ci started

@mbed-ci
Copy link

mbed-ci commented Jan 19, 2019

Test run: SUCCESS

Summary: 11 of 11 test jobs passed
Build number : 1
Build artifacts

@0xc0170
Copy link
Contributor

0xc0170 commented Jan 21, 2019

@SeppoTakalo
@marcuschangarm
@karsev

Any of you to review this patch ? It's ready for integration

@VeijoPesonen
Copy link
Contributor Author

VeijoPesonen commented Jan 21, 2019

Adding the non-blocking connect itself. @kjbracey-arm, @michalpasztamobica, @SeppoTakalo please re-review.

@VeijoPesonen VeijoPesonen changed the title ESP8266: adds check for already established connection before new one ESP8266: implements possibility to decide between non-blocking/blocking connect. Jan 21, 2019
@0xc0170
Copy link
Contributor

0xc0170 commented Jan 21, 2019

CI started

Veijo Pesonen added 2 commits January 21, 2019 16:22
Blocking mode connect is not supported anymore when this patch is
applied
Implements NetworkInterface::set_blocking() and implements the
functionality to distinguish between the two in connect()
@mbed-ci
Copy link

mbed-ci commented Jan 21, 2019

Test run: FAILED

Summary: 3 of 7 test jobs failed
Build number : 2
Build artifacts

Failed test jobs:

  • jenkins-ci/mbed-os-ci_build-GCC_ARM
  • jenkins-ci/mbed-os-ci_build-IAR
  • jenkins-ci/mbed-os-ci_build-ARM

@0xc0170
Copy link
Contributor

0xc0170 commented Jan 21, 2019

CI aborted until review is completed

@0xc0170
Copy link
Contributor

0xc0170 commented Jan 22, 2019

CI started (travis restarted as well)

@mbed-ci
Copy link

mbed-ci commented Jan 22, 2019

Test run: FAILED

Summary: 2 of 11 test jobs failed
Build number : 3
Build artifacts

Failed test jobs:

  • jenkins-ci/mbed-os-ci_dynamic-memory-usage
  • jenkins-ci/mbed-os-ci_greentea-test

@cmonr
Copy link
Contributor

cmonr commented Jan 22, 2019

CI job restarted: jenkins-ci/mbed-os-ci_greentea-test

Failure appeared unrelated to PR.

@0xc0170
Copy link
Contributor

0xc0170 commented Jan 23, 2019

CI restarted

@cmonr cmonr merged commit f790fdd into ARMmbed:master Jan 24, 2019
@VeijoPesonen VeijoPesonen deleted the feature-esp8266_nonblocking_connect branch September 6, 2019 10:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants