-
Notifications
You must be signed in to change notification settings - Fork 7.3k
samples: add panther board sensor MQTT application #401
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
Conversation
samples/panther/ap/src/ipm.c
Outdated
{ | ||
int len = 0; | ||
|
||
len += sprintf(json+len, "{"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer that we use snprintf() or snprintk() (the latter uses less memory) which would check that you do not overwrite the buffer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better yet: use the JSON library that comes with Zephyr.
samples/panther/ap/src/ipm.c
Outdated
|
||
k_sem_init(&sync_sem, 0, 1); | ||
|
||
k_thread_spawn(ipm_stack, STACKSIZE, ipm_thread, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The k_thread_spawn() is deprecated, you should use k_thread_create() instead.
Note also that PR #392 (not yet merged) will bring some changes how the stack is defined.
samples/panther/ap/src/ipm.c
Outdated
sensors.BME280[2].val1 = (val->val1 * 10) + (val->val2 / 100000); | ||
sensors.BME280[2].val2 = (val->val2 % 100000) * 10; | ||
break; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor nit pick, the breaks in this switch do not look unified (some have empty line after and/or before). Could you make this look better in this respect?
samples/panther/ap/src/ipm.c
Outdated
static char ipm_stack[STACKSIZE]; | ||
static char json[2048]; | ||
static struct shield_sens sensors; | ||
static struct k_sem sync_sem; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
too many spaces
samples/panther/ap/src/mqtt.c
Outdated
|
||
#define ZEPHYR_ADDR "0.0.0.0" | ||
|
||
#define APP_SLEEP_MSECS 5000 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The values look weirdly aligned, perhaps use spaces here instead of tabs?
samples/panther/sensor/src/main.c
Outdated
} | ||
|
||
while (1) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This empty line is useless here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a nice example, but there are multiple issues with this patch; I didn't finish reviewing it.
samples/panther/ap/src/ipm.c
Outdated
{ | ||
int len = 0; | ||
|
||
len += sprintf(json+len, "{"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better yet: use the JSON library that comes with Zephyr.
samples/panther/ap/src/mqtt.c
Outdated
|
||
client_ctx = CONTAINER_OF(mqtt_ctx, struct mqtt_client_ctx, mqtt_ctx); | ||
|
||
printk("[%s:%d]", __func__, __LINE__); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd rather use the logging stuff for debugging messages. (There are other places in the code where printk() is used and the logging subsystem could be used instead.)
samples/panther/ap/src/mqtt.c
Outdated
*/ | ||
int mqtt_send(char *topic, char *payload) | ||
{ | ||
struct mqtt_publish_msg *pub_msg = &client_ctx.pub_msg; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indentation is wrong here. Please use tabs. (Add a git commit hook to always pass your changes through checkpatch. The documentation explains how to do it.)
samples/panther/ap/src/wifi.c
Outdated
|
||
ret = net_mgmt(NET_REQUEST_WIFI_CMD_AP_CONNECT, iface, | ||
&req, sizeof(req)); | ||
if(ret){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing spaces here: if (ret) {
. Please use checkpatch.
samples/panther/sensor/src/main.c
Outdated
/* fetch sensor samples */ | ||
old = NULL; | ||
for (i = 0; i < ARRAY_SIZE(info); i++) { | ||
if (info[i].dev) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better to do if (!info[i].dev]) { continue; }
here. Avoids one indentation level.
samples/panther/ap/src/ipm.c
Outdated
QUARK_SE_IPM_DEFINE(ess_ipm, 0, QUARK_SE_IPM_INBOUND); | ||
|
||
static char ipm_stack[STACKSIZE]; | ||
static char json[2048]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need for json
to be global. Define it where it's used, or better yet: use the JSON library that comes with Zephyr... it's safer.
4dd80d5
to
0f12ff6
Compare
Signed-off-by: Massimiliano Agneni <[email protected]> Signed-off-by: Anas Nashif <[email protected]>
…oject-rtos#401) Signed-off-by: Geoff Gustafson <[email protected]>
panther board was removed |
Signed-off-by: Massimiliano Agneni [email protected]