Removes global min/max definition & global std namespace selection

[timoxd7]

Removes global min/max definition causing problems with other libraries

Some libraries define min/max macros too (like STL) by using templates. With these makros already defined, the compilation might fail or have an unexprected behaviour.

Removes using namespace std from header file

Inlcuding using namespace std in a header file results in a using namespace std be part of any header/cpp file using this header. This might lead to unexpected behaviour and false selection of methods/functions by setting the std namespace to global scope.

[CLAassistant]

All committers have signed the CLA.

[rahult-github]

Hi @timoxd7 , Thanks for the contribution. However, all these changes are inside nimble stack. I suggest to raise the PR on upstream mynewt-nimble codebase here . ESP-IDF basically tries to maintain same code as upstream nimble. We can pull the commit in IDF once it gets merged in upstream code.

[AxelLin]

@rahult-github

Just curious if you will upgrade esp-nimble to nimble-1.5.0? (nimble-1.5.0 was tagged on 6 May)

[rahult-github]

@AxelLin , nimble-1.5.0 tag is created, but release branch is not yet created. We are internally preparing to sync to the new branch, but we would wait for the official release branch (mostly like 1_5_0_dev ) to be first available, before ESP-IDF starts on bringing the branch in official code on github.

[AxelLin]

@AxelLin , nimble-1.5.0 tag is created, but release branch is not yet created.

The NimBLE 1.5.0 was released quite long time ago, https://cwiki.apache.org/confluence/display/MYNEWT/RN-NimBLE-1.5.0 I check the nimble_1_3_0_dev and nimble_1_4_0_dev branches, there is no further commits after made 1.3.0/1.4.0 tag. (The nimble_1_4_0_dev branch looks like a dev branch for 1.4.0, which is to prepare 1.4.0 release and no longer update after made a release tag) Seems no point to wait for nimble_1_5_0_dev branch. I suspect they no longer create new dev branches, just use master tree now.

[timoxd7]

Made a pull request in apache/mynewt-nimble#1405

[AxelLin]

Hi @timoxd7 , Thanks for the contribution. However, all these changes are inside nimble stack. I suggest to raise the PR on upstream mynewt-nimble codebase here . ESP-IDF basically tries to maintain same code as upstream nimble. We can pull the commit in IDF once it gets merged in upstream code.

@rahult-github FYI, It's already merged by upstream mynewt-nimble.

[AxelLin]

@rahult-github

1. Would you pull this pull request? (It's already merged in upstream per your request).
2. Would you upgrade esp-nimble to nimble-1.5.0? As I mentioned NimBLE 1.5.0 was released quite long time ago. There is no further commit after nimble_1_4_0_dev tag in nimble_1_4_0_dev branch, so I see no benefit waitting for 1_5_0_dev branch. If you are still waiting for 1_5_0_dev branch, please ask it on their maillist.

[rahult-github]

Hi @AxelLin ,

1. Change is queued up at our end for merging. Will show up on github soon once it passes the internal checks and procedures.
2. nimble-1.5.0 introduces a new transport layer which has part functionality in host and some in controller. We are working on merging the branch, but since it requires some design change for our code too , hence the delay. Request you to please bear with us for this request.

Thanks

[rahult-github]

Change is now part of master branch. Will soon be moved to other applicable branches as per procedure.

[AxelLin]

@rahult-github

It's not clear to me about this change: (Why always setting from_ll = true; ? Is this intentional or a mistake?) https://github.com/espressif/esp-nimble/commit/86007550e226fd810135558c2aa08acbdc602e95#diff-971b17b68b5cb5a1a185e74e952575cd93ded97c3a0505f85ac09acec579ec5dR166

And not sure why BLE_TRANSPORT_ACL_FROM_LL_COUNT is renamed to BLE_TRANSPORT_ACL_FROM_HS_COUNT. https://github.com/espressif/esp-nimble/commit/86007550e226fd810135558c2aa08acbdc602e95#diff-a2e7b946885e9634b32809a44646a8cc551f082b88b14677231c14a6bd581f5eL43-R146

[RoshanESP]

Hi @AxelLin , Thanks for pointing that out. Changes were made to make it work with Espressif chips, we will make the code generic though.

[AxelLin]

@RoshanESP

Compare to upstream nimble-1.5.0. The esp-nimble has a lot of below checking in various functions: if (!ble_hs_is_enabled()) { return BLE_HS_EDISABLED; }

If these calls are indeeded fixes to nimble, why not sending PR to upstream?

[AxelLin]

Hi @AxelLin , Thanks for pointing that out. Changes were made to make it work with Espressif chips, we will make the code generic though.

For "always setting from_ll = true;", I think you need to add a comment about why you need to do this and also set proper #ifdef guard so it has no impact if espressif bt chips are not used.

For rename BLE_TRANSPORT_ACL_FROM_LL_COUNT to BLE_TRANSPORT_ACL_FROM_HS_COUNT, actually I think you simply cannot do this because it now read different settings from upstream. And I don't understand what is achieved with this rename.

[RoshanESP]

Hi @AxelLin ,

1. For "always setting from_ll = true;", I think you need to add a comment about why you need to do this and also set proper #ifdef guard so it has no impact if espressif bt chips are not used. This change is removed internally and the MR is raised regarding the same.
2. For rename BLE_TRANSPORT_ACL_FROM_LL_COUNT to BLE_TRANSPORT_ACL_FROM_HS_COUNT, actually I think you simply cannot do this because it now read different settings from upstream. This is also fixed internally and changes will be soon visible on GitHub.

@RoshanESP

Compare to upstream nimble-1.5.0. The esp-nimble has a lot of below checking in various functions: if (!ble_hs_is_enabled()) { return BLE_HS_EDISABLED; }

If these calls are indeeded fixes to nimble, why not sending PR to upstream?

I agree. We will work on this and raise PR on upstream.

[AxelLin]

@rahult-github @RoshanESP

FYI, NimBLE 1.6.0 released https://lists.apache.org/thread/l0s3fml3m4ncfg9sw9gkhtgkzq94fnmd