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/29 07:23:01 UTC
[GitHub] [incubator-nuttx-apps] SPRESENSE opened a new pull request #346: Update wget
SPRESENSE opened a new pull request #346:
URL: https://github.com/apache/incubator-nuttx-apps/pull/346
## Summary
Improve examples/wget
- Enable URL from command line
- It can be used with ifup command
- Add HTTPS protocol
## Impact
only wget
## Testing
Tested by spresense:wifi
----------------------------------------------------------------
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] SPRESENSE commented on a change in pull request #346: Update wget
Posted by GitBox <gi...@apache.org>.
SPRESENSE commented on a change in pull request #346:
URL: https://github.com/apache/incubator-nuttx-apps/pull/346#discussion_r471187159
##########
File path: examples/wget/Kconfig
##########
@@ -12,11 +12,18 @@ config EXAMPLES_WGET
if EXAMPLES_WGET
+config EXAMPLES_WGET_STACKSIZE
+ int "wget stack size"
+ default 2048
+ ---help---
+ Stack size of "wget" command. If you use SSL connection,
+ you should increase this value around 8096.
+
config EXAMPLES_WGET_URL
Review comment:
Is it possible to use both of command line and option? I found it used in some defconfigs.
----------------------------------------------------------------
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] SPRESENSE commented on a change in pull request #346: Update wget
Posted by GitBox <gi...@apache.org>.
SPRESENSE commented on a change in pull request #346:
URL: https://github.com/apache/incubator-nuttx-apps/pull/346#discussion_r471854907
##########
File path: examples/wget/Kconfig
##########
@@ -12,11 +12,18 @@ config EXAMPLES_WGET
if EXAMPLES_WGET
+config EXAMPLES_WGET_STACKSIZE
+ int "wget stack size"
+ default 2048
+ ---help---
+ Stack size of "wget" command. If you use SSL connection,
+ you should increase this value around 8096.
+
config EXAMPLES_WGET_URL
string "File URL"
default ""
---help---
- The URL of the file to get
+ The URL of the file to get
config EXAMPLES_WGET_NOMAC
Review comment:
I see. I've fixed it and main code.
----------------------------------------------------------------
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 #346: Update wget
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #346:
URL: https://github.com/apache/incubator-nuttx-apps/pull/346#discussion_r471200521
##########
File path: examples/wget/Kconfig
##########
@@ -12,11 +12,18 @@ config EXAMPLES_WGET
if EXAMPLES_WGET
+config EXAMPLES_WGET_STACKSIZE
+ int "wget stack size"
+ default 2048
Review comment:
Yes, but it's better to change 2048 to DEFAULT_TASK_STACKSIZE, so we can chagne all program stack size(e.g. sim).
----------------------------------------------------------------
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 #346: Update wget
Posted by GitBox <gi...@apache.org>.
yamt commented on a change in pull request #346:
URL: https://github.com/apache/incubator-nuttx-apps/pull/346#discussion_r466848860
##########
File path: netutils/webclient/webclient.c
##########
@@ -658,12 +658,18 @@ static int wget_base(FAR const char *url, FAR char *buffer, int buflen,
len = dest - buffer;
- ret = send(sockfd, buffer, len, 0);
- if (ret < 0)
+ do
{
- nerr("ERROR: send failed: %d\n", errno);
- goto errout;
+ ret = send(sockfd, buffer + ((dest - buffer) - len), len, 0);
+ if (ret < 0)
+ {
+ nerr("ERROR: send failed: %d\n", errno);
+ goto errout;
+ }
+
+ len -= ret;
}
+ while (len > 0);
Review comment:
does this mean short-writes on a blocking socket?
----------------------------------------------------------------
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 #346: Update wget
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #346:
URL: https://github.com/apache/incubator-nuttx-apps/pull/346#issuecomment-675536316
LGTM.
----------------------------------------------------------------
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 #346: Update wget
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #346:
URL: https://github.com/apache/incubator-nuttx-apps/pull/346#discussion_r467858455
##########
File path: examples/wget/Kconfig
##########
@@ -12,11 +12,18 @@ config EXAMPLES_WGET
if EXAMPLES_WGET
+config EXAMPLES_WGET_STACKSIZE
+ int "wget stack size"
+ default 2048
Review comment:
should we use DEFAULT_TASK_STACKSIZE 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] SPRESENSE commented on a change in pull request #346: Update wget
Posted by GitBox <gi...@apache.org>.
SPRESENSE commented on a change in pull request #346:
URL: https://github.com/apache/incubator-nuttx-apps/pull/346#discussion_r471845688
##########
File path: examples/wget/Kconfig
##########
@@ -12,11 +12,18 @@ config EXAMPLES_WGET
if EXAMPLES_WGET
+config EXAMPLES_WGET_STACKSIZE
+ int "wget stack size"
+ default 2048
+ ---help---
+ Stack size of "wget" command. If you use SSL connection,
+ you should increase this value around 8096.
+
config EXAMPLES_WGET_URL
Review comment:
Thanks. I'll fix it to both.
----------------------------------------------------------------
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 #346: Update wget
Posted by GitBox <gi...@apache.org>.
yamt commented on pull request #346:
URL: https://github.com/apache/incubator-nuttx-apps/pull/346#issuecomment-664926912
----------------------------------------------------------------
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] SPRESENSE commented on a change in pull request #346: Update wget
Posted by GitBox <gi...@apache.org>.
SPRESENSE commented on a change in pull request #346:
URL: https://github.com/apache/incubator-nuttx-apps/pull/346#discussion_r471185279
##########
File path: examples/wget/Kconfig
##########
@@ -12,11 +12,18 @@ config EXAMPLES_WGET
if EXAMPLES_WGET
+config EXAMPLES_WGET_STACKSIZE
+ int "wget stack size"
+ default 2048
+ ---help---
+ Stack size of "wget" command. If you use SSL connection,
+ you should increase this value around 8096.
+
config EXAMPLES_WGET_URL
string "File URL"
default ""
---help---
- The URL of the file to get
+ The URL of the file to get
config EXAMPLES_WGET_NOMAC
Review comment:
I have no idea.
It looks that not related with NSH_NETINIT, isn't 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
[GitHub] [incubator-nuttx-apps] SPRESENSE commented on a change in pull request #346: Update wget
Posted by GitBox <gi...@apache.org>.
SPRESENSE commented on a change in pull request #346:
URL: https://github.com/apache/incubator-nuttx-apps/pull/346#discussion_r471399608
##########
File path: examples/wget/Kconfig
##########
@@ -12,11 +12,18 @@ config EXAMPLES_WGET
if EXAMPLES_WGET
+config EXAMPLES_WGET_STACKSIZE
+ int "wget stack size"
+ default 2048
Review comment:
I misunderstood. I'll fix 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
[GitHub] [incubator-nuttx-apps] xiaoxiang781216 merged pull request #346: Update wget
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged pull request #346:
URL: https://github.com/apache/incubator-nuttx-apps/pull/346
----------------------------------------------------------------
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 #346: Update wget
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #346:
URL: https://github.com/apache/incubator-nuttx-apps/pull/346#discussion_r471201267
##########
File path: examples/wget/Kconfig
##########
@@ -12,11 +12,18 @@ config EXAMPLES_WGET
if EXAMPLES_WGET
+config EXAMPLES_WGET_STACKSIZE
+ int "wget stack size"
+ default 2048
+ ---help---
+ Stack size of "wget" command. If you use SSL connection,
+ you should increase this value around 8096.
+
config EXAMPLES_WGET_URL
Review comment:
But your patch don't reference EXAMPLES_WGET_URL anymore and the code return -1 if user don't pass a URL. So please make your code and Kconfig consistent with each other.
----------------------------------------------------------------
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] SPRESENSE commented on a change in pull request #346: Update wget
Posted by GitBox <gi...@apache.org>.
SPRESENSE commented on a change in pull request #346:
URL: https://github.com/apache/incubator-nuttx-apps/pull/346#discussion_r471394813
##########
File path: examples/wget/Kconfig
##########
@@ -12,11 +12,18 @@ config EXAMPLES_WGET
if EXAMPLES_WGET
+config EXAMPLES_WGET_STACKSIZE
+ int "wget stack size"
+ default 2048
+ ---help---
+ Stack size of "wget" command. If you use SSL connection,
+ you should increase this value around 8096.
+
config EXAMPLES_WGET_URL
Review comment:
Sorry, my previous comment was mistake.
I know this patch is wrong.
I think it has 2 ways for fix it. 1) completely remove EXAMPLES_WGET_URL option and 2) to be able to use both of command line and option.
Do you think which better ones?
----------------------------------------------------------------
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] SPRESENSE commented on a change in pull request #346: Update wget
Posted by GitBox <gi...@apache.org>.
SPRESENSE commented on a change in pull request #346:
URL: https://github.com/apache/incubator-nuttx-apps/pull/346#discussion_r471182499
##########
File path: examples/wget/Kconfig
##########
@@ -12,11 +12,18 @@ config EXAMPLES_WGET
if EXAMPLES_WGET
+config EXAMPLES_WGET_STACKSIZE
+ int "wget stack size"
+ default 2048
Review comment:
Actually, this patch is for SSL support, but useful in the future.
----------------------------------------------------------------
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 #346: Update wget
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #346:
URL: https://github.com/apache/incubator-nuttx-apps/pull/346#issuecomment-665081741
----------------------------------------------------------------
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 #346: Update wget
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #346:
URL: https://github.com/apache/incubator-nuttx-apps/pull/346#discussion_r471202071
##########
File path: examples/wget/Kconfig
##########
@@ -12,11 +12,18 @@ config EXAMPLES_WGET
if EXAMPLES_WGET
+config EXAMPLES_WGET_STACKSIZE
+ int "wget stack size"
+ default 2048
+ ---help---
+ Stack size of "wget" command. If you use SSL connection,
+ you should increase this value around 8096.
+
config EXAMPLES_WGET_URL
string "File URL"
default ""
---help---
- The URL of the file to get
+ The URL of the file to get
config EXAMPLES_WGET_NOMAC
Review comment:
I mean change to:
```
config EXAMPLES_WGET_NOMAC
depends on !NSH_NETINIT
```
It doesn't make sense to let user select mac related option if NSH_NETINIT turn on because your code skip all related code in this case.
----------------------------------------------------------------
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 #346: Update wget
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #346:
URL: https://github.com/apache/incubator-nuttx-apps/pull/346#discussion_r467860603
##########
File path: examples/wget/Kconfig
##########
@@ -12,11 +12,18 @@ config EXAMPLES_WGET
if EXAMPLES_WGET
+config EXAMPLES_WGET_STACKSIZE
+ int "wget stack size"
+ default 2048
+ ---help---
+ Stack size of "wget" command. If you use SSL connection,
+ you should increase this value around 8096.
+
config EXAMPLES_WGET_URL
string "File URL"
default ""
---help---
- The URL of the file to get
+ The URL of the file to get
config EXAMPLES_WGET_NOMAC
Review comment:
Should we guard this by NSH_NETINIT?
----------------------------------------------------------------
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 #346: Update wget
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #346:
URL: https://github.com/apache/incubator-nuttx-apps/pull/346#discussion_r467860150
##########
File path: examples/wget/Kconfig
##########
@@ -12,11 +12,18 @@ config EXAMPLES_WGET
if EXAMPLES_WGET
+config EXAMPLES_WGET_STACKSIZE
+ int "wget stack size"
+ default 2048
+ ---help---
+ Stack size of "wget" command. If you use SSL connection,
+ you should increase this value around 8096.
+
config EXAMPLES_WGET_URL
Review comment:
should we remve this option? since it get from command line 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] SPRESENSE commented on pull request #346: Update wget
Posted by GitBox <gi...@apache.org>.
SPRESENSE commented on pull request #346:
URL: https://github.com/apache/incubator-nuttx-apps/pull/346#issuecomment-670335064
@yamt
Sorry too late.
I removed SSL support patch, because you already pushed.
However, I have some questions for your patch, maybe I'll comment on #333 later.
@xiaoxiang781216
Could you please review again?
----------------------------------------------------------------
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 #346: Update wget
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #346:
URL: https://github.com/apache/incubator-nuttx-apps/pull/346#discussion_r471429393
##########
File path: examples/wget/Kconfig
##########
@@ -12,11 +12,18 @@ config EXAMPLES_WGET
if EXAMPLES_WGET
+config EXAMPLES_WGET_STACKSIZE
+ int "wget stack size"
+ default 2048
+ ---help---
+ Stack size of "wget" command. If you use SSL connection,
+ you should increase this value around 8096.
+
config EXAMPLES_WGET_URL
Review comment:
Both are fine for me.
----------------------------------------------------------------
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] SPRESENSE commented on a change in pull request #346: Update wget
Posted by GitBox <gi...@apache.org>.
SPRESENSE commented on a change in pull request #346:
URL: https://github.com/apache/incubator-nuttx-apps/pull/346#discussion_r471182083
##########
File path: netutils/webclient/webclient.c
##########
@@ -658,12 +658,18 @@ static int wget_base(FAR const char *url, FAR char *buffer, int buflen,
len = dest - buffer;
- ret = send(sockfd, buffer, len, 0);
- if (ret < 0)
+ do
{
- nerr("ERROR: send failed: %d\n", errno);
- goto errout;
+ ret = send(sockfd, buffer + ((dest - buffer) - len), len, 0);
+ if (ret < 0)
+ {
+ nerr("ERROR: send failed: %d\n", errno);
+ goto errout;
+ }
+
+ len -= ret;
}
+ while (len > 0);
Review comment:
Yes.
----------------------------------------------------------------
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