You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2021/03/28 23:56:46 UTC
[GitHub] [incubator-nuttx-apps] Ouss4 opened a new pull request #636: Support for MQTT-C
Ouss4 opened a new pull request #636:
URL: https://github.com/apache/incubator-nuttx-apps/pull/636
## Summary
Add glue code for MQTT-C and an example to use the library.
## Impact
No impact on existing code. Library disabled by default.
## Testing
Tested with ESP32 DevkitC and Olimex STM32 P407. defconfigs attached, IP addresses needs to be adapted.
[esp32_mqttc.txt](https://github.com/apache/incubator-nuttx-apps/files/6218337/esp32_mqttc.txt)
[olimex_mqttc.txt](https://github.com/apache/incubator-nuttx-apps/files/6218338/olimex_mqttc.txt)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on a change in pull request #636: Support for MQTT-C
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #636:
URL: https://github.com/apache/incubator-nuttx-apps/pull/636#discussion_r603117475
##########
File path: examples/mqttc/Kconfig
##########
@@ -0,0 +1,31 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfig-language.txt in the NuttX tools repository.
+#
+
+config EXAMPLES_MQTTC
+ tristate "Enable MQTT-C Example"
+ default n
+ depends on NETUTILS_MQTTC
+ ---help---
+ Enable a simple MQTT-C publisher example
+
+if EXAMPLES_MQTTC
+
+config EXAMPLES_MQTTC_PROGNAME
+ string "Program name"
+ default "mqttc_pub"
+
+config EXAMPLES_MQTTC_STACKSIZE
+ int "Task's stack size"
+ default 8192
Review comment:
Can we use the default stack size(2KB) here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] Ouss4 commented on pull request #636: Support for MQTT-C
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on pull request #636:
URL: https://github.com/apache/incubator-nuttx-apps/pull/636#issuecomment-808985309
Mixed Case style error comes from MQTT-C APIs.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] Ouss4 commented on a change in pull request #636: Support for MQTT-C
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #636:
URL: https://github.com/apache/incubator-nuttx-apps/pull/636#discussion_r603124454
##########
File path: examples/mqttc/Kconfig
##########
@@ -0,0 +1,31 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfig-language.txt in the NuttX tools repository.
+#
+
+config EXAMPLES_MQTTC
+ tristate "Enable MQTT-C Example"
+ default n
+ depends on NETUTILS_MQTTC
+ ---help---
+ Enable a simple MQTT-C publisher example
+
+if EXAMPLES_MQTTC
+
+config EXAMPLES_MQTTC_PROGNAME
+ string "Program name"
+ default "mqttc_pub"
+
+config EXAMPLES_MQTTC_STACKSIZE
+ int "Task's stack size"
+ default 8192
Review comment:
I'm not sure if that's enough. I remember that I had to adjust the stack size a while back when I had this example. I'll do some tests later today and see if 2KB is enough.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] Ouss4 commented on a change in pull request #636: Support for MQTT-C
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #636:
URL: https://github.com/apache/incubator-nuttx-apps/pull/636#discussion_r603125392
##########
File path: mqtt/mqttc/Make.defs
##########
@@ -0,0 +1,32 @@
+############################################################################
+# apps/mqtt/mqttc/Make.defs
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership. The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+ifneq ($(CONFIG_MQTT_MQTTC),)
+CONFIGURED_APPS += $(APPDIR)/mqtt/mqttc
+
+CFLAGS += -D__unix__
+CXXFLAGS += -D__unix__
Review comment:
@xiaoxiang781216 tried to move them (as is done in other places) but they don't seem to take any effect in Makefile. Any thing to look for here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] Ouss4 commented on a change in pull request #636: Support for MQTT-C
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #636:
URL: https://github.com/apache/incubator-nuttx-apps/pull/636#discussion_r602953342
##########
File path: mqtt/mqttc/Make.defs
##########
@@ -0,0 +1,32 @@
+############################################################################
+# apps/mqtt/mqttc/Make.defs
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership. The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+ifneq ($(CONFIG_MQTT_MQTTC),)
+CONFIGURED_APPS += $(APPDIR)/mqtt/mqttc
+
+CFLAGS += -D__unix__
+CXXFLAGS += -D__unix__
Review comment:
Should we contribute `__NuttX__` to the upstream library instead? We've got similar situations with other 3rd party software as well. [mbedTLS](https://github.com/apache/incubator-nuttx-apps/pull/566/files#diff-6043c39303f7b14be00e75ffcac170a85c3ee0b7369e7db85f4131ef85280c81R39) is one of them.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] v01d commented on pull request #636: Support for MQTT-C
Posted by GitBox <gi...@apache.org>.
v01d commented on pull request #636:
URL: https://github.com/apache/incubator-nuttx-apps/pull/636#issuecomment-808990370
Thanks for submitting. I will test it on sim if I can (otherwise on ESP32).
BTW, we discussed putting the library under netutils on the list, could you move it there so that we not crowd to toplevel directory?
I think this will make the include path change to `netutils/mqtt.h`, unless we leave it in top-level include for compatibility.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] v01d commented on a change in pull request #636: Support for MQTT-C
Posted by GitBox <gi...@apache.org>.
v01d commented on a change in pull request #636:
URL: https://github.com/apache/incubator-nuttx-apps/pull/636#discussion_r602956778
##########
File path: mqtt/mqttc/Make.defs
##########
@@ -0,0 +1,32 @@
+############################################################################
+# apps/mqtt/mqttc/Make.defs
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership. The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+ifneq ($(CONFIG_MQTT_MQTTC),)
+CONFIGURED_APPS += $(APPDIR)/mqtt/mqttc
+
+CFLAGS += -D__unix__
+CXXFLAGS += -D__unix__
Review comment:
Yeah, I think it is a simple change to request. In the meantime we can leave this definition.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on a change in pull request #636: Support for MQTT-C
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #636:
URL: https://github.com/apache/incubator-nuttx-apps/pull/636#discussion_r603044377
##########
File path: mqtt/mqttc/Kconfig
##########
@@ -0,0 +1,18 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfig-language.txt in the NuttX tools repository.
+#
+
+config MQTT_MQTTC
Review comment:
change MQTT_MQTTC to MQTTC?
##########
File path: mqtt/mqttc/Kconfig
##########
@@ -0,0 +1,18 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfig-language.txt in the NuttX tools repository.
+#
+
+config MQTT_MQTTC
Review comment:
should we add NETUTILS_ prefix?
##########
File path: mqtt/mqttc/Make.defs
##########
@@ -0,0 +1,32 @@
+############################################################################
+# apps/mqtt/mqttc/Make.defs
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership. The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+ifneq ($(CONFIG_MQTT_MQTTC),)
+CONFIGURED_APPS += $(APPDIR)/mqtt/mqttc
+
+CFLAGS += -D__unix__
+CXXFLAGS += -D__unix__
Review comment:
```__unix__``` is only used in implementation, it's better to move CLFAGS/CXXFLAGS into Makefile
##########
File path: mqtt/mqttc/Make.defs
##########
@@ -0,0 +1,32 @@
+############################################################################
+# apps/mqtt/mqttc/Make.defs
Review comment:
change path to netutils/mqttc?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on a change in pull request #636: Support for MQTT-C
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #636:
URL: https://github.com/apache/incubator-nuttx-apps/pull/636#discussion_r603137851
##########
File path: mqtt/mqttc/Make.defs
##########
@@ -0,0 +1,32 @@
+############################################################################
+# apps/mqtt/mqttc/Make.defs
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership. The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+ifneq ($(CONFIG_MQTT_MQTTC),)
+CONFIGURED_APPS += $(APPDIR)/mqtt/mqttc
+
+CFLAGS += -D__unix__
+CXXFLAGS += -D__unix__
Review comment:
I just create a patch to mainline: https://github.com/LiamBindle/MQTT-C/pull/131
We can remove these lines once the patch get merged, so let's ignore it now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] xiaoxiang781216 merged pull request #636: Support for MQTT-C
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged pull request #636:
URL: https://github.com/apache/incubator-nuttx-apps/pull/636
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] Ouss4 commented on pull request #636: Support for MQTT-C
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on pull request #636:
URL: https://github.com/apache/incubator-nuttx-apps/pull/636#issuecomment-809231621
I updated the defconfigs in the description to account for the new location.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on a change in pull request #636: Support for MQTT-C
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #636:
URL: https://github.com/apache/incubator-nuttx-apps/pull/636#discussion_r603137851
##########
File path: mqtt/mqttc/Make.defs
##########
@@ -0,0 +1,32 @@
+############################################################################
+# apps/mqtt/mqttc/Make.defs
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership. The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+ifneq ($(CONFIG_MQTT_MQTTC),)
+CONFIGURED_APPS += $(APPDIR)/mqtt/mqttc
+
+CFLAGS += -D__unix__
+CXXFLAGS += -D__unix__
Review comment:
I just create a patch to mainline: https://github.com/LiamBindle/MQTT-C/pull/131
We can remove these lines once the patch get merged, let's ignore it now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] Ouss4 commented on a change in pull request #636: Support for MQTT-C
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #636:
URL: https://github.com/apache/incubator-nuttx-apps/pull/636#discussion_r603140208
##########
File path: mqtt/mqttc/Make.defs
##########
@@ -0,0 +1,32 @@
+############################################################################
+# apps/mqtt/mqttc/Make.defs
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership. The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+ifneq ($(CONFIG_MQTT_MQTTC),)
+CONFIGURED_APPS += $(APPDIR)/mqtt/mqttc
+
+CFLAGS += -D__unix__
+CXXFLAGS += -D__unix__
Review comment:
But we'll still need to wait for the next release. Or use master?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx-apps] xiaoxiang781216 commented on a change in pull request #636: Support for MQTT-C
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #636:
URL: https://github.com/apache/incubator-nuttx-apps/pull/636#discussion_r603123832
##########
File path: mqtt/mqttc/Make.defs
##########
@@ -0,0 +1,32 @@
+############################################################################
+# apps/mqtt/mqttc/Make.defs
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership. The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+############################################################################
+
+ifneq ($(CONFIG_MQTT_MQTTC),)
+CONFIGURED_APPS += $(APPDIR)/mqtt/mqttc
+
+CFLAGS += -D__unix__
+CXXFLAGS += -D__unix__
Review comment:
@Ouss4 how about this comment?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org