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 2020/07/12 00:01:01 UTC
[GitHub] [incubator-nuttx-apps] yamt opened a new pull request #333: webclient improvements
yamt opened a new pull request #333:
URL: https://github.com/apache/incubator-nuttx-apps/pull/333
Highlights:
* TLS support (a hook to allow users to provide TLS implementation)
* ability to add extra request headers
* ability to use PUT method
* ability to report http status
* error handling improvements
Proposed on the ML while ago:
https://www.mail-archive.com/dev@nuttx.apache.org/msg03803.html
The original API is kept for now.
I plan to remove them after adapting the existing users.
(examples in this repo)
## Summary
## Impact
## Testing
----------------------------------------------------------------
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] yamt commented on pull request #333: webclient improvements
Posted by GitBox <gi...@apache.org>.
yamt commented on pull request #333:
URL: https://github.com/apache/incubator-nuttx-apps/pull/333#issuecomment-662766290
@xiaoxiang781216 no. i plan to work on this occasionally and step-by-step.
----------------------------------------------------------------
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] yamt commented on a change in pull request #333: webclient improvements
Posted by GitBox <gi...@apache.org>.
yamt commented on a change in pull request #333:
URL: https://github.com/apache/incubator-nuttx-apps/pull/333#discussion_r481982448
##########
File path: include/netutils/webclient.h
##########
@@ -147,6 +281,12 @@ int wget(FAR const char *url, FAR char *buffer, int buflen,
int wget_post(FAR const char *url, FAR const char *posts, FAR char *buffer,
int buflen, wget_callback_t callback, FAR void *arg);
+void webclient_set_defaults(FAR struct webclient_context *ctx);
Review comment:
if you prefer step-by-step, this PR (the first step) is ready to review/merge already.
----------------------------------------------------------------
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] yamt commented on pull request #333: webclient improvements
Posted by GitBox <gi...@apache.org>.
yamt commented on pull request #333:
URL: https://github.com/apache/incubator-nuttx-apps/pull/333#issuecomment-701877486
@xiaoxiang781216 i plan to remove wget/wget_base. but not in this PR.
----------------------------------------------------------------
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] yamt commented on a change in pull request #333: webclient improvements
Posted by GitBox <gi...@apache.org>.
yamt commented on a change in pull request #333:
URL: https://github.com/apache/incubator-nuttx-apps/pull/333#discussion_r456970063
##########
File path: include/netutils/webclient.h
##########
@@ -147,6 +281,12 @@ int wget(FAR const char *url, FAR char *buffer, int buflen,
int wget_post(FAR const char *url, FAR const char *posts, FAR char *buffer,
int buflen, wget_callback_t callback, FAR void *arg);
+void webclient_set_defaults(FAR struct webclient_context *ctx);
Review comment:
i plan to remove all wget_ things after adapting users.
----------------------------------------------------------------
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] acassis commented on pull request #333: webclient improvements
Posted by GitBox <gi...@apache.org>.
acassis commented on pull request #333:
URL: https://github.com/apache/incubator-nuttx-apps/pull/333#issuecomment-684806475
Hi @yamt, don't hurry up. I'm just ping to know the status.
----------------------------------------------------------------
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] jerpelea commented on a change in pull request #333: webclient improvements
Posted by GitBox <gi...@apache.org>.
jerpelea commented on a change in pull request #333:
URL: https://github.com/apache/incubator-nuttx-apps/pull/333#discussion_r480897547
##########
File path: include/netutils/webclient.h
##########
@@ -147,6 +281,12 @@ int wget(FAR const char *url, FAR char *buffer, int buflen,
int wget_post(FAR const char *url, FAR const char *posts, FAR char *buffer,
int buflen, wget_callback_t callback, FAR void *arg);
+void webclient_set_defaults(FAR struct webclient_context *ctx);
Review comment:
@yamt please do it step by step
----------------------------------------------------------------
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 pull request #333: webclient improvements
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #333:
URL: https://github.com/apache/incubator-nuttx-apps/pull/333#issuecomment-701947934
OK, let's merge this one first.
----------------------------------------------------------------
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] acassis commented on a change in pull request #333: webclient improvements
Posted by GitBox <gi...@apache.org>.
acassis commented on a change in pull request #333:
URL: https://github.com/apache/incubator-nuttx-apps/pull/333#discussion_r479383605
##########
File path: include/netutils/webclient.h
##########
@@ -147,6 +281,12 @@ int wget(FAR const char *url, FAR char *buffer, int buflen,
int wget_post(FAR const char *url, FAR const char *posts, FAR char *buffer,
int buflen, wget_callback_t callback, FAR void *arg);
+void webclient_set_defaults(FAR struct webclient_context *ctx);
Review comment:
ping @yamt any news?
----------------------------------------------------------------
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] yamt commented on a change in pull request #333: webclient improvements
Posted by GitBox <gi...@apache.org>.
yamt commented on a change in pull request #333:
URL: https://github.com/apache/incubator-nuttx-apps/pull/333#discussion_r480821686
##########
File path: include/netutils/webclient.h
##########
@@ -147,6 +281,12 @@ int wget(FAR const char *url, FAR char *buffer, int buflen,
int wget_post(FAR const char *url, FAR const char *posts, FAR char *buffer,
int buflen, wget_callback_t callback, FAR void *arg);
+void webclient_set_defaults(FAR struct webclient_context *ctx);
Review comment:
@acassis
no news. are you waiting for this PR "complete"?
like adapting users, removing the old api, etc, in a single shot?
i prefer to work on a step-by-step way.
(as a bonus, it might give external users time to adapt their code to the new api.)
but if you prefer a single-shot, i can try.
----------------------------------------------------------------
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 pull request #333: webclient improvements
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #333:
URL: https://github.com/apache/incubator-nuttx-apps/pull/333#issuecomment-662498523
@yamt basically, the ideal is good, do you plan to upstream all part together?
----------------------------------------------------------------
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] yamt commented on a change in pull request #333: webclient improvements
Posted by GitBox <gi...@apache.org>.
yamt commented on a change in pull request #333:
URL: https://github.com/apache/incubator-nuttx-apps/pull/333#discussion_r456969662
##########
File path: include/netutils/webclient.h
##########
@@ -97,6 +97,140 @@
typedef void (*wget_callback_t)(FAR char **buffer, int offset,
int datend, FAR int *buflen, FAR void *arg);
+/* webclient_sink_callback_t: callback to consume data
+ *
+ * Same as wget_callback_t, but allowed to fail.
+ *
+ * Input Parameters:
+ * Same as wget_callback_t
+ *
+ * Return value:
+ * 0 on success.
+ * A negative errno on error.
+ */
+
+typedef CODE int (*webclient_sink_callback_t)(FAR char **buffer, int offset,
Review comment:
yes. wget_callback_t is only for compat. i plan to remove wget_callback_t after adapting all users.
----------------------------------------------------------------
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] yamt commented on pull request #333: webclient improvements
Posted by GitBox <gi...@apache.org>.
yamt commented on pull request #333:
URL: https://github.com/apache/incubator-nuttx-apps/pull/333#issuecomment-701217278
the following nxstyle complaints are expected. (cJSON)
examples/wgetjson/wgetjson_main.c:184:2: error: Mixed case identifier found
examples/wgetjson/wgetjson_main.c:186:9: error: Mixed case identifier found
examples/wgetjson/wgetjson_main.c:189:37: error: Mixed case identifier found
examples/wgetjson/wgetjson_main.c:193:12: error: Mixed case identifier found
examples/wgetjson/wgetjson_main.c:194:6: error: Mixed case identifier found
examples/wgetjson/wgetjson_main.c:206:39: error: Mixed case identifier found
examples/wgetjson/wgetjson_main.c:259:36: error: Mixed case identifier found
examples/wgetjson/wgetjson_main.c:287:2: error: Mixed case identifier found
examples/wgetjson/wgetjson_main.c:290:9: error: Mixed case identifier found
examples/wgetjson/wgetjson_main.c:293:37: error: Mixed case identifier found
examples/wgetjson/wgetjson_main.c:299:6: error: Mixed case identifier found
----------------------------------------------------------------
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 #333: webclient improvements
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged pull request #333:
URL: https://github.com/apache/incubator-nuttx-apps/pull/333
----------------------------------------------------------------
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 #333: webclient improvements
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #333:
URL: https://github.com/apache/incubator-nuttx-apps/pull/333#discussion_r456772641
##########
File path: include/netutils/webclient.h
##########
@@ -97,6 +97,140 @@
typedef void (*wget_callback_t)(FAR char **buffer, int offset,
int datend, FAR int *buflen, FAR void *arg);
+/* webclient_sink_callback_t: callback to consume data
+ *
+ * Same as wget_callback_t, but allowed to fail.
+ *
+ * Input Parameters:
+ * Same as wget_callback_t
+ *
+ * Return value:
+ * 0 on success.
+ * A negative errno on error.
+ */
+
+typedef CODE int (*webclient_sink_callback_t)(FAR char **buffer, int offset,
Review comment:
Will you merge wget_callback_t and webclient_sink_callback_t into one finally?
##########
File path: include/netutils/webclient.h
##########
@@ -147,6 +281,12 @@ int wget(FAR const char *url, FAR char *buffer, int buflen,
int wget_post(FAR const char *url, FAR const char *posts, FAR char *buffer,
int buflen, wget_callback_t callback, FAR void *arg);
+void webclient_set_defaults(FAR struct webclient_context *ctx);
Review comment:
Should we unify the prefix? either wget_xxx or webclient_xxx, but not both.
##########
File path: include/netutils/webclient.h
##########
@@ -97,6 +97,140 @@
typedef void (*wget_callback_t)(FAR char **buffer, int offset,
int datend, FAR int *buflen, FAR void *arg);
+/* webclient_sink_callback_t: callback to consume data
+ *
+ * Same as wget_callback_t, but allowed to fail.
+ *
+ * Input Parameters:
+ * Same as wget_callback_t
+ *
+ * Return value:
+ * 0 on success.
+ * A negative errno on error.
+ */
+
+typedef CODE int (*webclient_sink_callback_t)(FAR char **buffer, int offset,
+ int datend, FAR int *buflen,
+ FAR void *arg);
+
+/* webclient_body_callback_t: a callback to provide request body
+ *
+ * This callback can be called multiple times to provide
+ * webclient_context::bodylen bytes of the data.
+ * It's the responsibility of this callback to maintain the current
+ * "offset" of the data. (similarly to fread(3))
+ *
+ * An implementation of this callback should perform either of
+ * the followings:
+ *
+ * - fill the buffer (specified by buffer and *sizep) with the data
+ * - update *datap with a buffer filled with the data
+ *
+ * Either ways, it should update *sizep to the size of the data.
+ *
+ * A short result is allowed. In that case, this callback will be called
+ * again to provide the remaining data.
+ *
+ * Input Parameters:
+ * buffer - The buffer to fill.
+ * sizep - The size of buffer/data in bytes.
+ * datap - The data to return.
Review comment:
Why need datap if we already has buffer?
----------------------------------------------------------------
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] yamt commented on pull request #333: webclient improvements
Posted by GitBox <gi...@apache.org>.
yamt commented on pull request #333:
URL: https://github.com/apache/incubator-nuttx-apps/pull/333#issuecomment-701108053
i will adapt wgetjson once https://github.com/apache/incubator-nuttx-apps/pull/411 is merged
----------------------------------------------------------------
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] yamt commented on pull request #333: webclient improvements
Posted by GitBox <gi...@apache.org>.
yamt commented on pull request #333:
URL: https://github.com/apache/incubator-nuttx-apps/pull/333#issuecomment-701217543
I adapted all users of wget/wget_post in the tree to the new api.
----------------------------------------------------------------
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] yamt commented on a change in pull request #333: webclient improvements
Posted by GitBox <gi...@apache.org>.
yamt commented on a change in pull request #333:
URL: https://github.com/apache/incubator-nuttx-apps/pull/333#discussion_r456969960
##########
File path: include/netutils/webclient.h
##########
@@ -97,6 +97,140 @@
typedef void (*wget_callback_t)(FAR char **buffer, int offset,
int datend, FAR int *buflen, FAR void *arg);
+/* webclient_sink_callback_t: callback to consume data
+ *
+ * Same as wget_callback_t, but allowed to fail.
+ *
+ * Input Parameters:
+ * Same as wget_callback_t
+ *
+ * Return value:
+ * 0 on success.
+ * A negative errno on error.
+ */
+
+typedef CODE int (*webclient_sink_callback_t)(FAR char **buffer, int offset,
+ int datend, FAR int *buflen,
+ FAR void *arg);
+
+/* webclient_body_callback_t: a callback to provide request body
+ *
+ * This callback can be called multiple times to provide
+ * webclient_context::bodylen bytes of the data.
+ * It's the responsibility of this callback to maintain the current
+ * "offset" of the data. (similarly to fread(3))
+ *
+ * An implementation of this callback should perform either of
+ * the followings:
+ *
+ * - fill the buffer (specified by buffer and *sizep) with the data
+ * - update *datap with a buffer filled with the data
+ *
+ * Either ways, it should update *sizep to the size of the data.
+ *
+ * A short result is allowed. In that case, this callback will be called
+ * again to provide the remaining data.
+ *
+ * Input Parameters:
+ * buffer - The buffer to fill.
+ * sizep - The size of buffer/data in bytes.
+ * datap - The data to return.
Review comment:
to allow users to pass their own buffers directly.
the existing wget_callback_t prototype made me think it can be important for some users to avoid extra buffers.
i myself don't need it.
----------------------------------------------------------------
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