Re: [PR] Add missing variables and change to be more intuitive to use [nuttx-apps]
GC-20-20 closed pull request #2171: Add missing variables and change to be more intuitive to use URL: https://github.com/apache/nuttx-apps/pull/2171 -- 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. To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Add missing variables and change to be more intuitive to use [nuttx-apps]
xiaoxiang781216 commented on code in PR #2171:
URL: https://github.com/apache/nuttx-apps/pull/2171#discussion_r1381796313
##
examples/serialrx/serialrx_main.c:
##
@@ -173,7 +172,6 @@ int main(int argc, FAR char *argv[])
UNUSED(eof);
}
-
Review Comment:
revert this change
##
examples/serialrx/serialrx_main.c:
##
@@ -118,8 +118,7 @@ int main(int argc, FAR char *argv[])
while (cnt < bytecount)
{
#ifdef CONFIG_EXAMPLES_SERIALRX_BUFFERED
- size_t n = fread(buf, 1, 26, f);
- cnt++;
+ size_t n = fread(buf, 1, 1, f);
Review Comment:
why not write 26 bytes.
--
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.
To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Add missing variables and change to be more intuitive to use [nuttx-apps]
xiaoxiang781216 commented on PR #2171: URL: https://github.com/apache/nuttx-apps/pull/2171#issuecomment-1792568419 please squash your patch and update the commit message to reflect your change. -- 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. To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Add missing variables and change to be more intuitive to use [nuttx-apps]
xiaoxiang781216 commented on code in PR #2171:
URL: https://github.com/apache/nuttx-apps/pull/2171#discussion_r1378622996
##
examples/serialrx/serialrx_main.c:
##
@@ -118,8 +118,7 @@ int main(int argc, FAR char *argv[])
while (cnt < bytecount)
{
#ifdef CONFIG_EXAMPLES_SERIALRX_BUFFERED
- size_t n = fread(buf, 1, 26, f);
Review Comment:
You can have one PR which contain more than one patch
--
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.
To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Add missing variables and change to be more intuitive to use [nuttx-apps]
GC-20-20 commented on code in PR #2171:
URL: https://github.com/apache/nuttx-apps/pull/2171#discussion_r1378414801
##
examples/serialrx/serialrx_main.c:
##
@@ -118,8 +118,7 @@ int main(int argc, FAR char *argv[])
while (cnt < bytecount)
{
#ifdef CONFIG_EXAMPLES_SERIALRX_BUFFERED
- size_t n = fread(buf, 1, 26, f);
Review Comment:
Okay, I see what you mean.I'd still like to ask, but would these small
changes be a waste of your time.After all, it doesn't change the logic of the
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.
To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Add missing variables and change to be more intuitive to use [nuttx-apps]
xiaoxiang781216 commented on code in PR #2171:
URL: https://github.com/apache/nuttx-apps/pull/2171#discussion_r1378410290
##
examples/serialrx/serialrx_main.c:
##
@@ -118,8 +118,7 @@ int main(int argc, FAR char *argv[])
while (cnt < bytecount)
{
#ifdef CONFIG_EXAMPLES_SERIALRX_BUFFERED
- size_t n = fread(buf, 1, 26, f);
Review Comment:
Ok, but after you finish the investigation is better to remove the temp
change. If you think the temp change is worth to upstream, please split to the
different patch. It's important to ensure one patch fix one problem, so the
reviewer could understand your change quickly.
--
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.
To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Add missing variables and change to be more intuitive to use [nuttx-apps]
GC-20-20 commented on code in PR #2171:
URL: https://github.com/apache/nuttx-apps/pull/2171#discussion_r1378380224
##
examples/serialrx/serialrx_main.c:
##
@@ -118,8 +118,7 @@ int main(int argc, FAR char *argv[])
while (cnt < bytecount)
{
#ifdef CONFIG_EXAMPLES_SERIALRX_BUFFERED
- size_t n = fread(buf, 1, 26, f);
Review Comment:
I'm very sorry, this change is really optional, this 26 I double-checked a
few times without finding any clues, then I changed it to a simple number.
Maybe it helps.
--
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.
To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Add missing variables and change to be more intuitive to use [nuttx-apps]
GC-20-20 commented on code in PR #2171: URL: https://github.com/apache/nuttx-apps/pull/2171#discussion_r1378378099 ## examples/serialrx/Kconfig: ## @@ -39,22 +39,22 @@ config EXAMPLES_SERIALRX_DEVPATH choice prompt "Output method" - default EXAMPLES_SERIALRX_PRINTHYPHEN + default EXAMPLES_SERIALRX_PRINTSTR Review Comment: When I opened this application for the first time, it was very confusing for me to not have read the source code of the program and not have any display after using another serial port to send data, after all, there is no explanation in the HELP. So I would like to change the default value to EXAMPLES_SERIALRX_PRINTSTR so that later learners can be more intuitively clear that the previous operation is correct. -- 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. To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Add missing variables and change to be more intuitive to use [nuttx-apps]
xiaoxiang781216 commented on code in PR #2171:
URL: https://github.com/apache/nuttx-apps/pull/2171#discussion_r1378373428
##
examples/serialrx/Kconfig:
##
@@ -39,22 +39,22 @@ config EXAMPLES_SERIALRX_DEVPATH
choice
prompt "Output method"
- default EXAMPLES_SERIALRX_PRINTHYPHEN
+ default EXAMPLES_SERIALRX_PRINTSTR
-config EXAMPLES_SERIALRX_PRINTHYPHEN
Review Comment:
why change the order, let's keep as before
##
examples/serialrx/serialrx_main.c:
##
@@ -118,8 +118,7 @@ int main(int argc, FAR char *argv[])
while (cnt < bytecount)
{
#ifdef CONFIG_EXAMPLES_SERIALRX_BUFFERED
- size_t n = fread(buf, 1, 26, f);
Review Comment:
why make the follow change
##
examples/serialrx/Kconfig:
##
@@ -39,22 +39,22 @@ config EXAMPLES_SERIALRX_DEVPATH
choice
prompt "Output method"
- default EXAMPLES_SERIALRX_PRINTHYPHEN
+ default EXAMPLES_SERIALRX_PRINTSTR
Review Comment:
why change the default setting
--
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.
To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[PR] Add missing variables and change to be more intuitive to use [nuttx-apps]
GC-20-20 opened a new pull request, #2171: URL: https://github.com/apache/nuttx-apps/pull/2171 ## Summary Add missing variables and change to be more intuitive to use ## Impact Variables that do cause compilation to fail ## Testing local test -- 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. To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
