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

included assert.h to Thingsboard.h because of compilation error for MKR GSM 1400 #137

Merged
merged 6 commits into from
May 17, 2023

Conversation

NHuhs1887
Copy link
Contributor

@NHuhs1887 NHuhs1887 commented May 14, 2023

Because of a compilation error I get, when compiling the library for my Arduino mkr gsm 1400, I included the assert.h.
The error message was the following:

`In file included from C:\Users\Niklas\OneDrive\Dokumente\Arduino\Rewako_3\Rewako_3.ino:12:0:
C:\Users\Niklas\OneDrive\Dokumente\Arduino\libraries\ThingsBoard\src/ThingsBoard.h: In static member function 'static uint8_t ThingsBoardSized<MaxFieldsAmt, Logger>::detectSize(const char*, ...)':
C:\Users\Niklas\OneDrive\Dokumente\Arduino\libraries\ThingsBoard\src/ThingsBoard.h:1341:7: error: there are no arguments to 'assert' that depend on a template parameter, so a declaration of 'assert' must be available [-fpermissive]
assert(result >= 0);
^~~~~~
C:\Users\Niklas\OneDrive\Dokumente\Arduino\libraries\ThingsBoard\src/ThingsBoard.h:1341:7: note: (if you use '-fpermissive', G++ will accept your code, but allowing the use of an undeclared name is deprecated)
C:\Users\Niklas\OneDrive\Dokumente\Arduino\libraries\ThingsBoard\src/ThingsBoard.h: In instantiation of 'static uint8_t ThingsBoardSized<MaxFieldsAmt, Logger>::detectSize(const char*, ...) [with unsigned int MaxFieldsAmt = 8; Logger = ThingsBoardDefaultLogger; uint8_t = unsigned char]':
C:\Users\Niklas\OneDrive\Dokumente\Arduino\libraries\ThingsBoard\src/ThingsBoard.h:776:32: required from 'bool ThingsBoardSized<MaxFieldsAmt, Logger>::sendTelemetryJson(const ArduinoJson::V6212PB::JsonVariant&, const uint32_t&) [with unsigned int MaxFieldsAmt = 8; Logger = ThingsBoardDefaultLogger; uint32_t = long unsigned int]'
C:\Users\Niklas\OneDrive\Dokumente\Arduino\libraries\ThingsBoard\src/ThingsBoard.h:1871:43: required from 'bool ThingsBoardSized<MaxFieldsAmt, Logger>::sendKeyValue(const char*, T, bool) [with T = float; unsigned int MaxFieldsAmt = 8; Logger = ThingsBoardDefaultLogger]'
C:\Users\Niklas\OneDrive\Dokumente\Arduino\libraries\ThingsBoard\src/ThingsBoard.h:672:26: required from 'bool ThingsBoardSized<MaxFieldsAmt, Logger>::sendTelemetryFloat(const char*, float) [with unsigned int MaxFieldsAmt = 8; Logger = ThingsBoardDefaultLogger]'
C:\Users\Niklas\OneDrive\Dokumente\Arduino\Rewako_3\Rewako_3.ino:287:45: required from here
C:\Users\Niklas\OneDrive\Dokumente\Arduino\libraries\ThingsBoard\src/ThingsBoard.h:1341:13: error: 'assert' was not declared in this scope
assert(result >= 0);
~~~~~~^~~~~~~~~~~~~
C:\Users\Niklas\OneDrive\Dokumente\Arduino\libraries\ThingsBoard\src/ThingsBoard.h:1341:13: note: suggested alternative: 'ushort'
assert(result >= 0);
~~~~~~^~~~~~~~~~~~~
ushort

exit status 1

Compilation error: exit status 1`

After including assert.h in Thingsboard.h, the error is gone and the library works as expected.

Copy link
Contributor

@MathewHDYT MathewHDYT left a comment

Choose a reason for hiding this comment

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

The include position is good and seems fine, but it only needs to be included / can most of the time be included if C++ STL is supported therefore a #ifdef around the #include needs to be added.

@@ -10,6 +10,8 @@
// Library includes.
#include <stdarg.h>
#include <TBPubSubClient.h>
#include <assert.h>
Copy link
Contributor

Choose a reason for hiding this comment

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

#if THINGSBOARD_ENABLE_STL
#include <assert.h>
#endif // THINGSBOARD_ENABLE_STL

Copy link
Contributor Author

Choose a reason for hiding this comment

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

tried it and got the mentioned error again.

Copy link
Contributor

Choose a reason for hiding this comment

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

That include now has to be moved below Configuration.h, because in that file the THINGSBOARD_ENABLE_STL is defined.
Additionally the extra spaces are not needed, 2 spaces to private and then another 2 is done in other code files in the library as well.

Would be nice if you could revert the latest commit and then add this code below the local includes.

#if THINGSBOARD_ENABLE_STL
#include <assert.h>
#endif // THINGSBOARD_ENABLE_STL

Copy link
Contributor

Choose a reason for hiding this comment

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

@NHuhs1887 Would be nice if the changes requested above could be done as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure. Will get to it this afternoon.

@NHuhs1887
Copy link
Contributor Author

@MathewHDYT is that alright?

@MathewHDYT
Copy link
Contributor

MathewHDYT commented May 16, 2023

Seems good thanks a lot for the pull request. Contributions are always welcome :D.

This pull request can be merged @imbeacon, before you create a new release tough would be nice if you could wait shortly. I will create a PR for another issue #136 and increase the version package numbers and then the current library state can be released with both bugfixes as v0.10.2.

@MathewHDYT
Copy link
Contributor

Pull request has been created as #138

@imbeacon imbeacon merged commit 8e368f4 into thingsboard:master May 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants