Skip to content

Add support for publishing individual records #47

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

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

nvelozsavino
Copy link

calling ts_txt_statement on an ts_object pointing a simple item now adds to the buffer the item with the format #
Ex:
#/foo/rBar 23

Copy link
Contributor

@martinjaeger martinjaeger left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution. That's a useful update.

Got two small remarks.

if (len_value <= 0) {
return 0;
}
len+=len_value;
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add two spaces for consistent code format:

Suggested change
len+=len_value;
len += len_value;

Comment on lines +1068 to +1070
if (len >= buf_size - 1) {
return 0;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this check is not necessary as the maximum available size was already provided to ts_json_serialize_value before. So if no error occurred, the data will always fit into the buffer.

Copy link
Contributor

@martinjaeger martinjaeger left a comment

Choose a reason for hiding this comment

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

Thanks for the update. Looks good from function point of view. There are only some small style issues left and the unit test has to be enabled.

In order to actually execute the new unit test, it has to be added to zephyr/tests/src/main.c (for Zephyr) and test/main.cpp for PlatformIO.

Can you please make the coding style consistent with the rest of the code (especially regarding spaces). The style is following the Libre Solar style.

@@ -118,6 +118,8 @@ static int json_serialize_simple_value(char *buf, size_t size, void *data, int t
return snprintf(buf, size, "%" PRIu8 ",", *((uint8_t *)data));
case TS_T_INT8:
return snprintf(buf, size, "%" PRIi8 ",", *((int8_t *)data));
case TS_T_BOOL:
Copy link
Contributor

Choose a reason for hiding this comment

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

TS_T_BOOL is already covered. See line 135.


// TXT STATEMTENT DATA ///////////////////////////////////////////////////////

TS_GROUP(ID_TXT,"TXT",TS_NO_CALLBACK,ID_ROOT),
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
TS_GROUP(ID_TXT,"TXT",TS_NO_CALLBACK,ID_ROOT),
TS_GROUP(ID_TXT, "TXT", TS_NO_CALLBACK, ID_ROOT),

Comment on lines +240 to +241


Copy link
Contributor

Choose a reason for hiding this comment

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

Unnecessary newlines.

{"TXT/rBoolFalse","false"},
};

for (size_t i=0;i<ARRAY_SIZE(tests);i++){
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
for (size_t i=0;i<ARRAY_SIZE(tests);i++){
for (size_t i = 0; i < ARRAY_SIZE(tests); i++) {

@martinjaeger
Copy link
Contributor

The unit tests are failing.

@martinjaeger
Copy link
Contributor

Unit tests are still failing because you added new data items which would have to be considered in the tests that discover all data objects. Not sure if adding new data items is really needed or if we can just test with the existing ones.

@martinjaeger
Copy link
Contributor

BTW: You can easily run the unit tests in your local Zephyr environment with the commands explained here: https://github.com/ThingSet/thingset-device-library/blob/main/zephyr/tests/README.md

@nvelozsavino nvelozsavino force-pushed the statement-simple-value branch from 145c21f to 0e93f34 Compare January 19, 2023 22:42
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.

2 participants