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

Changed returnCode enum fields to use the MQTT_ name prefix to avoid global namespace collisions #214

Open
wants to merge 2 commits into
base: master
from

Conversation

@al3xsh
Copy link

@al3xsh al3xsh commented Nov 10, 2020

When using the MQTTClient-C code with the STM32F7 series of microcontrollers (and the ST HAL libraries), there is a global namespace collision between the returnCode enum field in MQTTClient.h and the core stm32f7xx.h ErrorStatus enum. Best practice would be to use a name prefix to avoid these global namespace collisions so I've added one to the MQTTClient-C code.

Regards,

Alex

…global namespace collisions

Signed-off-by: Alex Shenfield <alex.shenfield@gmail.com>
@al3xsh al3xsh force-pushed the al3xsh:master branch from 2ad0c2f to 4fd80fe Nov 10, 2020
@scaprile
Copy link
Contributor

@scaprile scaprile commented Nov 10, 2020

This has already been reported on issue #191, and before that in issue #180.
My bet is that changing this would break working applications for all of us not using nor planning to ever use an STMwhatever, and many of us wouldn't like that.

Signed-off-by: Alex Shenfield <alex.shenfield@gmail.com>
@al3xsh
Copy link
Author

@al3xsh al3xsh commented Nov 11, 2020

Hi @scaprile,

Thanks for pointing those out. Despite searching the issues I managed to totally miss both of those.

You are probably correct - however, using such common enum values will keep causing problems for people (and not just with the ST libraries, but with use code too). To me it is somewhat indicative of "code smell" (this isn't singling out MQTTClient-C -> there are much more problems within the ST libraries themselves!).

My hope is that this change will be at least picked up upstream (for example in the Keil MDK-Packs bundle of eclipse-paho) in situations where it will cause problems.

Regards,

Alex

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.