Skip to content

Remove ns_event_loop_thread_start() from NDInterface and ThreadInterface connect() #6981

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

Merged
merged 1 commit into from
May 25, 2018

Conversation

yogpan01
Copy link
Contributor

@yogpan01 yogpan01 commented May 22, 2018

Description

ns_event_loop_thread_start() is incorrectly used at connect() phase, the initial
setup is already done at init() phase and the eventloop thread is correctly initialized.

Also, the usage of ns_event_loop_thread_start() API should be behind MBED_CONF_NANOSTACK_HAL_EVENT_LOOP_DISPATCH_FROM_APPLICATION
flag as application can decide to use main thread for event loop, which will result in linker error for this API call in
case of ARMCC compiler.

Pull request type

[x] Fix
[ ] Refactor
[ ] New target
[ ] Feature
[ ] Breaking change

…ace connect().

ns_event_loop_thread_start() is incorrectly used at connect() phase, the initial
setup is already done at init() phase and the eventloop thread is correctly initialized.

Also, the usage of ns_event_loop_thread_start() API should be behind MBED_CONF_NANOSTACK_HAL_EVENT_LOOP_DISPATCH_FROM_APPLICATION
flag as application can decide to use main thread for event loop, which will result in linker error for this API call in
case of ARMCC compiler.
@yogpan01
Copy link
Contributor Author

@TeroJaasko @kjbracey-arm @SeppoTakalo Please review.

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.

I'd go further and strip it altogether - drop the definition and declaration in the header file.

But this'll do.

@yogpan01
Copy link
Contributor Author

@0xc0170 @adbridge please take this into MASTER base.

@adbridge
Copy link
Contributor

@yogpan01 please follow the correct instructions in the PR header. i.e. Do not change the structure of this header and please put an x in only one box. On this occasion I will fix it for you.

@TeroJaasko
Copy link
Contributor

Is the CI jammed in "Expected - Waiting for status to be reported" state, or is it really taking 3 days to run them?

@kjbracey
Copy link
Contributor

There's a big CI backlog for 5.9.0rc1 so this fix may have to wait until 5.9.1 or at least a later rc :( We're currently populating the 5.9.1 list.

@TeroJaasko
Copy link
Contributor

TeroJaasko commented May 25, 2018

@kjbracey-arm: Then we (as in Cloud client Lite) must skip the 5.9.0 release completely, as it does not compile. :-( It is really important for us to get this fixed, even if happened on a later 5.9.0-RC.

@0xc0170
Copy link
Contributor

0xc0170 commented May 25, 2018

Thanks @TeroJaasko , we will do our best to get this fix in 5.9.0

@cmonr
Copy link
Contributor

cmonr commented May 25, 2018

/morph build

@cmonr
Copy link
Contributor

cmonr commented May 25, 2018

Gonna rerun this due to a status update failure :(

/morph build

@mbed-ci
Copy link

mbed-ci commented May 25, 2018

Build : SUCCESS

Build number : 2156
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/6981/

Triggering tests

/morph test
/morph uvisor-test
/morph export-build
/morph mbed2-build

@mbed-ci
Copy link

mbed-ci commented May 25, 2018

@mbed-ci
Copy link

mbed-ci commented May 25, 2018

@cmonr cmonr merged commit e3aa140 into ARMmbed:master May 25, 2018
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