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 2022/01/20 17:28:16 UTC
[GitHub] [incubator-nuttx] fjpanag opened a new pull request #5295: sim: Added Kconfig option for UART buffer size.
fjpanag opened a new pull request #5295:
URL: https://github.com/apache/incubator-nuttx/pull/5295
## Summary
In sim, the UART ports had buffers with hard-coded size.
They were inadequate for my application, so I added a Kconfig option for this. The desired buffer size can now be selected.
I seized the opportunity to also improve the UART Kconfigs.
Most importantly I added help entries on what are the sim UARTs, and how they can be used (a topic that confused me a bit when I first tried this feature).
## Impact
I don't expect any impact on any existing system, other than the introduction of the option `SIM_UART`.
## Testing
Sim is building properly with `SIM_UART` disabled.
I also enabled it, and tested it on a custom application that uses the UARTs *and* needs larger buffers.
Sim built and run exactly as expected. It was able to communicate with an external software and take advantage of the larger buffers.
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] fjpanag commented on a change in pull request #5295: sim: Added Kconfig option for UART buffer size.
Posted by GitBox <gi...@apache.org>.
fjpanag commented on a change in pull request #5295:
URL: https://github.com/apache/incubator-nuttx/pull/5295#discussion_r789067805
##########
File path: arch/sim/Kconfig
##########
@@ -422,28 +422,92 @@ endchoice
endif
+menuconfig SIM_UART
Review comment:
Yes that is very easy. But I did it like that because it looked much cleaner and provided a good point to add the "help" entry.
If I change it to a menu, where do you propose to move the help, so it is easily reachable by the 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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #5295: sim: Added Kconfig option for UART buffer size.
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5295:
URL: https://github.com/apache/incubator-nuttx/pull/5295#discussion_r788998791
##########
File path: arch/sim/Kconfig
##########
@@ -422,28 +422,92 @@ endchoice
endif
+menuconfig SIM_UART
Review comment:
can we change to menu? so we still use SIM_UART_NUMBER == 0 to disable uart simulation.
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #5295: sim: Added Kconfig option for UART buffer size.
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5295:
URL: https://github.com/apache/incubator-nuttx/pull/5295#discussion_r790857009
##########
File path: arch/sim/src/sim/up_uart.c
##########
@@ -31,12 +31,16 @@
#include "up_internal.h"
-#if defined(USE_DEVCONSOLE) || CONFIG_SIM_UART_NUMBER > 0
+#if defined(USE_DEVCONSOLE) || defined(CONFIG_SIM_UART)
/****************************************************************************
* Pre-processor Definitions
****************************************************************************/
-#define BUFSIZE 256
+#ifdef CONFIG_SIM_UART_BUFFER_SIZE
Review comment:
let's change to:
```
#define BUFSIZE CONFIG_SIM_UART_BUFFER_SIZE
```
or use CONFIG_SIM_UART_BUFFER_SIZE directly?
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #5295: sim: Added Kconfig option for UART buffer size.
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5295:
URL: https://github.com/apache/incubator-nuttx/pull/5295#discussion_r789309105
##########
File path: arch/sim/Kconfig
##########
@@ -422,28 +422,92 @@ endchoice
endif
+menuconfig SIM_UART
Review comment:
Still put here like "Simulated Graphics/Input" at line 253.
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] fjpanag commented on a change in pull request #5295: sim: Added Kconfig option for UART buffer size.
Posted by GitBox <gi...@apache.org>.
fjpanag commented on a change in pull request #5295:
URL: https://github.com/apache/incubator-nuttx/pull/5295#discussion_r790577728
##########
File path: arch/sim/src/sim/up_uart.c
##########
@@ -31,12 +31,16 @@
#include "up_internal.h"
-#if defined(USE_DEVCONSOLE) || CONFIG_SIM_UART_NUMBER > 0
+#if defined(USE_DEVCONSOLE) || defined(CONFIG_SIM_UART)
/****************************************************************************
* Pre-processor Definitions
****************************************************************************/
-#define BUFSIZE 256
+#ifdef CONFIG_SIM_UART_BUFFER_SIZE
Review comment:
I wouldn't like to do it like this. I believe that `CONFIG_` macros shall only be defined in Kconfigs, otherwise it will be a mess.
As I see, `BUFSIZE` is also used when `USE_DEVCONSOLE` is selected, even if no UARTs are configured (and thus `CONFIG_SIM_UART_BUFFER_SIZE` is not set).
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] jerpelea commented on a change in pull request #5295: sim: Added Kconfig option for UART buffer size.
Posted by GitBox <gi...@apache.org>.
jerpelea commented on a change in pull request #5295:
URL: https://github.com/apache/incubator-nuttx/pull/5295#discussion_r791871531
##########
File path: arch/sim/src/sim/up_uart.c
##########
@@ -31,12 +31,16 @@
#include "up_internal.h"
-#if defined(USE_DEVCONSOLE) || CONFIG_SIM_UART_NUMBER > 0
+#if defined(USE_DEVCONSOLE) || defined(CONFIG_SIM_UART)
/****************************************************************************
* Pre-processor Definitions
****************************************************************************/
-#define BUFSIZE 256
+#ifdef CONFIG_SIM_UART_BUFFER_SIZE
Review comment:
@fjpanag please change to
#ifndef CONFIG_SIM_UART_BUFFER_SIZE
# define CONFIG_SIM_UART_BUFFER_SIZE 256
#endif
as @pkarashchenko suggested
there is no need to keep the BUFSIZE
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] acassis commented on a change in pull request #5295: sim: Added Kconfig option for UART buffer size.
Posted by GitBox <gi...@apache.org>.
acassis commented on a change in pull request #5295:
URL: https://github.com/apache/incubator-nuttx/pull/5295#discussion_r791681134
##########
File path: arch/sim/Kconfig
##########
@@ -422,28 +422,87 @@ endchoice
endif
+menu "Simulated UART"
+
config SIM_UART_NUMBER
- int "The number of tty ports on sim platform, range is 0~4"
+ int "Number of simulated UART ports"
default 0
+ range 0 4
+ ---help---
+ Under simulation, a NuttX port can be bound to a serial
+ port on the host machine. This way NuttX can access the
+ host's hardware directly.
+
+ The NuttX and the host serial ports must have the same
+ name for this to work.
+
+ The host's port is not necessary to be an actual h/w port.
+ You can create a "simulated" port in your host too,
+ by running:
+
+ socat PTY,link=/dev/ttySIM0 PTY,link=/dev/ttyNX0
+ stty -F /dev/ttySIM0 raw
+ stty -F /dev/ttyNX0 raw
+
+ In this case NuttX will use the ttySIM0 port, and another
+ software may open and use the ttyNX0 port.
+ This way other softwares may communicate directly with
+ the simulation.
Review comment:
@fjpanag I think the "--help--" here should also show how the users can use a physical port (i.e. /dev/ttyUSB0). Although you said "can be bound to a serial port on the host machine" and that the port don't need to be an actual h/w port it will be confuse to newcomers to realize they can use a physical port because all the default names are to /dev/ttySIM0. I suggest start showing that he could use the physical port before creating new virtual ports using socat
##########
File path: arch/sim/Kconfig
##########
@@ -422,28 +422,87 @@ endchoice
endif
+menu "Simulated UART"
+
config SIM_UART_NUMBER
- int "The number of tty ports on sim platform, range is 0~4"
+ int "Number of simulated UART ports"
default 0
+ range 0 4
+ ---help---
+ Under simulation, a NuttX port can be bound to a serial
+ port on the host machine. This way NuttX can access the
+ host's hardware directly.
+
+ The NuttX and the host serial ports must have the same
+ name for this to work.
+
+ The host's port is not necessary to be an actual h/w port.
+ You can create a "simulated" port in your host too,
+ by running:
+
+ socat PTY,link=/dev/ttySIM0 PTY,link=/dev/ttyNX0
+ stty -F /dev/ttySIM0 raw
+ stty -F /dev/ttyNX0 raw
+
+ In this case NuttX will use the ttySIM0 port, and another
+ software may open and use the ttyNX0 port.
+ This way other softwares may communicate directly with
+ the simulation.
Review comment:
@fjpanag I think the "--help--" here should also show how the users can use a physical port (i.e. /dev/ttyUSB0). Although you said "can be bound to a serial port on the host machine" and that the port doesn't need to be an actual h/w port it will be confuse to newcomers to realize they can use a physical port because all the default names are to /dev/ttySIM0. I suggest start showing that he could use the physical port before creating new virtual ports using socat
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] fjpanag commented on a change in pull request #5295: sim: Added Kconfig option for UART buffer size.
Posted by GitBox <gi...@apache.org>.
fjpanag commented on a change in pull request #5295:
URL: https://github.com/apache/incubator-nuttx/pull/5295#discussion_r789547721
##########
File path: arch/sim/Kconfig
##########
@@ -422,28 +422,92 @@ endchoice
endif
+menuconfig SIM_UART
Review comment:
@xiaoxiang781216 I don't get it. "Simulated Graphics/Input" does not have a help entry.
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a change in pull request #5295: sim: Added Kconfig option for UART buffer size.
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5295:
URL: https://github.com/apache/incubator-nuttx/pull/5295#discussion_r790643872
##########
File path: arch/sim/src/sim/up_uart.c
##########
@@ -31,12 +31,16 @@
#include "up_internal.h"
-#if defined(USE_DEVCONSOLE) || CONFIG_SIM_UART_NUMBER > 0
+#if defined(USE_DEVCONSOLE) || defined(CONFIG_SIM_UART)
/****************************************************************************
* Pre-processor Definitions
****************************************************************************/
-#define BUFSIZE 256
+#ifdef CONFIG_SIM_UART_BUFFER_SIZE
Review comment:
Can we still go with
```
#ifndef CONFIG_SIM_UART_BUFFER_SIZE
# define CONFIG_SIM_UART_BUFFER_SIZE 256
#endif
```
??
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] acassis commented on a change in pull request #5295: sim: Added Kconfig option for UART buffer size.
Posted by GitBox <gi...@apache.org>.
acassis commented on a change in pull request #5295:
URL: https://github.com/apache/incubator-nuttx/pull/5295#discussion_r791681134
##########
File path: arch/sim/Kconfig
##########
@@ -422,28 +422,87 @@ endchoice
endif
+menu "Simulated UART"
+
config SIM_UART_NUMBER
- int "The number of tty ports on sim platform, range is 0~4"
+ int "Number of simulated UART ports"
default 0
+ range 0 4
+ ---help---
+ Under simulation, a NuttX port can be bound to a serial
+ port on the host machine. This way NuttX can access the
+ host's hardware directly.
+
+ The NuttX and the host serial ports must have the same
+ name for this to work.
+
+ The host's port is not necessary to be an actual h/w port.
+ You can create a "simulated" port in your host too,
+ by running:
+
+ socat PTY,link=/dev/ttySIM0 PTY,link=/dev/ttyNX0
+ stty -F /dev/ttySIM0 raw
+ stty -F /dev/ttyNX0 raw
+
+ In this case NuttX will use the ttySIM0 port, and another
+ software may open and use the ttyNX0 port.
+ This way other softwares may communicate directly with
+ the simulation.
Review comment:
@fjpanag I think the "--help--" here should also show how the users can use a physical port (i.e. /dev/ttyUSB0). Although you said "can be bound to a serial port on the host machine" and that the port don't need to be an actual h/w port it will be confuse to newcomers to realize they can use a physical port because all the default names are to /dev/ttySIM0. I suggest start showing that he could use the physical port before creating new virtual ports using socat
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] jerpelea commented on a change in pull request #5295: sim: Added Kconfig option for UART buffer size.
Posted by GitBox <gi...@apache.org>.
jerpelea commented on a change in pull request #5295:
URL: https://github.com/apache/incubator-nuttx/pull/5295#discussion_r791871531
##########
File path: arch/sim/src/sim/up_uart.c
##########
@@ -31,12 +31,16 @@
#include "up_internal.h"
-#if defined(USE_DEVCONSOLE) || CONFIG_SIM_UART_NUMBER > 0
+#if defined(USE_DEVCONSOLE) || defined(CONFIG_SIM_UART)
/****************************************************************************
* Pre-processor Definitions
****************************************************************************/
-#define BUFSIZE 256
+#ifdef CONFIG_SIM_UART_BUFFER_SIZE
Review comment:
@fjpanag please change to
#ifndef CONFIG_SIM_UART_BUFFER_SIZE
# define CONFIG_SIM_UART_BUFFER_SIZE 256
#endif
as @pkarashchenko suggested
there is no need to keep the BUFSIZE
##########
File path: arch/sim/src/sim/up_uart.c
##########
@@ -31,12 +31,16 @@
#include "up_internal.h"
-#if defined(USE_DEVCONSOLE) || CONFIG_SIM_UART_NUMBER > 0
+#if defined(USE_DEVCONSOLE) || defined(CONFIG_SIM_UART)
/****************************************************************************
* Pre-processor Definitions
****************************************************************************/
-#define BUFSIZE 256
+#ifdef CONFIG_SIM_UART_BUFFER_SIZE
Review comment:
@fjpanag please change to
```
#ifndef CONFIG_SIM_UART_BUFFER_SIZE
# define CONFIG_SIM_UART_BUFFER_SIZE 256
#endif
```
as @pkarashchenko suggested
there is no need to keep the BUFSIZE
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a change in pull request #5295: sim: Added Kconfig option for UART buffer size.
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5295:
URL: https://github.com/apache/incubator-nuttx/pull/5295#discussion_r790679638
##########
File path: arch/sim/src/sim/up_uart.c
##########
@@ -31,12 +31,16 @@
#include "up_internal.h"
-#if defined(USE_DEVCONSOLE) || CONFIG_SIM_UART_NUMBER > 0
+#if defined(USE_DEVCONSOLE) || defined(CONFIG_SIM_UART)
/****************************************************************************
* Pre-processor Definitions
****************************************************************************/
-#define BUFSIZE 256
+#ifdef CONFIG_SIM_UART_BUFFER_SIZE
Review comment:
The benefit is to eliminate `BUFSIZE` and switch to one define instead of two
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] jerpelea commented on a change in pull request #5295: sim: Added Kconfig option for UART buffer size.
Posted by GitBox <gi...@apache.org>.
jerpelea commented on a change in pull request #5295:
URL: https://github.com/apache/incubator-nuttx/pull/5295#discussion_r791871531
##########
File path: arch/sim/src/sim/up_uart.c
##########
@@ -31,12 +31,16 @@
#include "up_internal.h"
-#if defined(USE_DEVCONSOLE) || CONFIG_SIM_UART_NUMBER > 0
+#if defined(USE_DEVCONSOLE) || defined(CONFIG_SIM_UART)
/****************************************************************************
* Pre-processor Definitions
****************************************************************************/
-#define BUFSIZE 256
+#ifdef CONFIG_SIM_UART_BUFFER_SIZE
Review comment:
@fjpanag please change to
```
#ifndef CONFIG_SIM_UART_BUFFER_SIZE
# define CONFIG_SIM_UART_BUFFER_SIZE 256
#endif
```
as @pkarashchenko suggested
there is no need to keep the BUFSIZE
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] fjpanag commented on a change in pull request #5295: sim: Added Kconfig option for UART buffer size.
Posted by GitBox <gi...@apache.org>.
fjpanag commented on a change in pull request #5295:
URL: https://github.com/apache/incubator-nuttx/pull/5295#discussion_r793467349
##########
File path: arch/sim/Kconfig
##########
@@ -422,28 +422,87 @@ endchoice
endif
+menu "Simulated UART"
+
config SIM_UART_NUMBER
- int "The number of tty ports on sim platform, range is 0~4"
+ int "Number of simulated UART ports"
default 0
+ range 0 4
+ ---help---
+ Under simulation, a NuttX port can be bound to a serial
+ port on the host machine. This way NuttX can access the
+ host's hardware directly.
+
+ The NuttX and the host serial ports must have the same
+ name for this to work.
+
+ The host's port is not necessary to be an actual h/w port.
+ You can create a "simulated" port in your host too,
+ by running:
+
+ socat PTY,link=/dev/ttySIM0 PTY,link=/dev/ttyNX0
+ stty -F /dev/ttySIM0 raw
+ stty -F /dev/ttyNX0 raw
+
+ In this case NuttX will use the ttySIM0 port, and another
+ software may open and use the ttyNX0 port.
+ This way other softwares may communicate directly with
+ the simulation.
Review comment:
OK, good point. I will enhance this section a bit.
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] acassis commented on a change in pull request #5295: sim: Added Kconfig option for UART buffer size.
Posted by GitBox <gi...@apache.org>.
acassis commented on a change in pull request #5295:
URL: https://github.com/apache/incubator-nuttx/pull/5295#discussion_r791681134
##########
File path: arch/sim/Kconfig
##########
@@ -422,28 +422,87 @@ endchoice
endif
+menu "Simulated UART"
+
config SIM_UART_NUMBER
- int "The number of tty ports on sim platform, range is 0~4"
+ int "Number of simulated UART ports"
default 0
+ range 0 4
+ ---help---
+ Under simulation, a NuttX port can be bound to a serial
+ port on the host machine. This way NuttX can access the
+ host's hardware directly.
+
+ The NuttX and the host serial ports must have the same
+ name for this to work.
+
+ The host's port is not necessary to be an actual h/w port.
+ You can create a "simulated" port in your host too,
+ by running:
+
+ socat PTY,link=/dev/ttySIM0 PTY,link=/dev/ttyNX0
+ stty -F /dev/ttySIM0 raw
+ stty -F /dev/ttyNX0 raw
+
+ In this case NuttX will use the ttySIM0 port, and another
+ software may open and use the ttyNX0 port.
+ This way other softwares may communicate directly with
+ the simulation.
Review comment:
@fjpanag I think the "--help--" here should also show how the users can use a physical port (i.e. /dev/ttyUSB0). Although you said "can be bound to a serial port on the host machine" and that the port doesn't need to be an actual h/w port it will be confuse to newcomers to realize they can use a physical port because all the default names are to /dev/ttySIM0. I suggest start showing that he could use the physical port before creating new virtual ports using socat
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a change in pull request #5295: sim: Added Kconfig option for UART buffer size.
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5295:
URL: https://github.com/apache/incubator-nuttx/pull/5295#discussion_r790315177
##########
File path: arch/sim/src/sim/up_uart.c
##########
@@ -31,12 +31,16 @@
#include "up_internal.h"
-#if defined(USE_DEVCONSOLE) || CONFIG_SIM_UART_NUMBER > 0
+#if defined(USE_DEVCONSOLE) || defined(CONFIG_SIM_UART)
/****************************************************************************
* Pre-processor Definitions
****************************************************************************/
-#define BUFSIZE 256
+#ifdef CONFIG_SIM_UART_BUFFER_SIZE
Review comment:
But seems that `CONFIG_SIM_UART_BUFFER_SIZE` now is always defined
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #5295: sim: Added Kconfig option for UART buffer size.
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5295:
URL: https://github.com/apache/incubator-nuttx/pull/5295#discussion_r789599673
##########
File path: arch/sim/Kconfig
##########
@@ -422,28 +422,92 @@ endchoice
endif
+menuconfig SIM_UART
Review comment:
Oh, you mean help text? I thought that menu also can have help string, if not we can add to SIM_UART_NUMBER?
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #5295: sim: Added Kconfig option for UART buffer size.
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5295:
URL: https://github.com/apache/incubator-nuttx/pull/5295#discussion_r791676894
##########
File path: arch/sim/src/sim/up_uart.c
##########
@@ -31,12 +31,16 @@
#include "up_internal.h"
-#if defined(USE_DEVCONSOLE) || CONFIG_SIM_UART_NUMBER > 0
+#if defined(USE_DEVCONSOLE) || defined(CONFIG_SIM_UART)
/****************************************************************************
* Pre-processor Definitions
****************************************************************************/
-#define BUFSIZE 256
+#ifdef CONFIG_SIM_UART_BUFFER_SIZE
Review comment:
@fjpanag do you want to apply the suggested 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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] fjpanag commented on a change in pull request #5295: sim: Added Kconfig option for UART buffer size.
Posted by GitBox <gi...@apache.org>.
fjpanag commented on a change in pull request #5295:
URL: https://github.com/apache/incubator-nuttx/pull/5295#discussion_r793483085
##########
File path: arch/sim/Kconfig
##########
@@ -422,28 +422,87 @@ endchoice
endif
+menu "Simulated UART"
+
config SIM_UART_NUMBER
- int "The number of tty ports on sim platform, range is 0~4"
+ int "Number of simulated UART ports"
default 0
+ range 0 4
+ ---help---
+ Under simulation, a NuttX port can be bound to a serial
+ port on the host machine. This way NuttX can access the
+ host's hardware directly.
+
+ The NuttX and the host serial ports must have the same
+ name for this to work.
+
+ The host's port is not necessary to be an actual h/w port.
+ You can create a "simulated" port in your host too,
+ by running:
+
+ socat PTY,link=/dev/ttySIM0 PTY,link=/dev/ttyNX0
+ stty -F /dev/ttySIM0 raw
+ stty -F /dev/ttyNX0 raw
+
+ In this case NuttX will use the ttySIM0 port, and another
+ software may open and use the ttyNX0 port.
+ This way other softwares may communicate directly with
+ the simulation.
Review comment:
@acassis Do you think it is more clear 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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko merged pull request #5295: sim: Added Kconfig option for UART buffer size.
Posted by GitBox <gi...@apache.org>.
pkarashchenko merged pull request #5295:
URL: https://github.com/apache/incubator-nuttx/pull/5295
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a change in pull request #5295: sim: Added Kconfig option for UART buffer size.
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5295:
URL: https://github.com/apache/incubator-nuttx/pull/5295#discussion_r790315067
##########
File path: arch/sim/src/sim/up_uart.c
##########
@@ -31,12 +31,16 @@
#include "up_internal.h"
-#if defined(USE_DEVCONSOLE) || CONFIG_SIM_UART_NUMBER > 0
+#if defined(USE_DEVCONSOLE) || defined(CONFIG_SIM_UART)
/****************************************************************************
* Pre-processor Definitions
****************************************************************************/
-#define BUFSIZE 256
+#ifdef CONFIG_SIM_UART_BUFFER_SIZE
Review comment:
Just as an alternative we can replace `BUFSIZE` with `CONFIG_SIM_UART_BUFFER_SIZE` and have here
```
#ifndef CONFIG_SIM_UART_BUFFER_SIZE
# define CONFIG_SIM_UART_BUFFER_SIZE 256
#endif
```
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] fjpanag commented on a change in pull request #5295: sim: Added Kconfig option for UART buffer size.
Posted by GitBox <gi...@apache.org>.
fjpanag commented on a change in pull request #5295:
URL: https://github.com/apache/incubator-nuttx/pull/5295#discussion_r789067805
##########
File path: arch/sim/Kconfig
##########
@@ -422,28 +422,92 @@ endchoice
endif
+menuconfig SIM_UART
Review comment:
Yes that is very easy. But I did it like that because it looked much cleaner and provided a good point to add the "help" entry.
If I change it to a menu, where do you propose to move the help, so it is easily reachable by the users?
Edit: "easily reachable", I mean "somewhere expected", that someone will actually look into.
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] fjpanag commented on a change in pull request #5295: sim: Added Kconfig option for UART buffer size.
Posted by GitBox <gi...@apache.org>.
fjpanag commented on a change in pull request #5295:
URL: https://github.com/apache/incubator-nuttx/pull/5295#discussion_r790646608
##########
File path: arch/sim/src/sim/up_uart.c
##########
@@ -31,12 +31,16 @@
#include "up_internal.h"
-#if defined(USE_DEVCONSOLE) || CONFIG_SIM_UART_NUMBER > 0
+#if defined(USE_DEVCONSOLE) || defined(CONFIG_SIM_UART)
/****************************************************************************
* Pre-processor Definitions
****************************************************************************/
-#define BUFSIZE 256
+#ifdef CONFIG_SIM_UART_BUFFER_SIZE
Review comment:
@pkarashchenko Sorry, I didn't understand.
What will be the benefit of this?
When needed, `CONFIG_SIM_UART_BUFFER_SIZE` is automatically defined by Kconfig, why we need this check?
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] fjpanag commented on a change in pull request #5295: sim: Added Kconfig option for UART buffer size.
Posted by GitBox <gi...@apache.org>.
fjpanag commented on a change in pull request #5295:
URL: https://github.com/apache/incubator-nuttx/pull/5295#discussion_r790575711
##########
File path: arch/sim/Kconfig
##########
@@ -422,28 +422,92 @@ endchoice
endif
+menuconfig SIM_UART
Review comment:
Yes, it was about the help text. As I see, there cannot be a help entry on menus. And I this new option was a good place to have the help entry.
I did the change per your request, which removes the new option, but with a slightly more obscured help entry.
If you think this is OK, you may merge it. I re-tested it, and it is OK.
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #5295: sim: Added Kconfig option for UART buffer size.
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #5295:
URL: https://github.com/apache/incubator-nuttx/pull/5295#discussion_r791676894
##########
File path: arch/sim/src/sim/up_uart.c
##########
@@ -31,12 +31,16 @@
#include "up_internal.h"
-#if defined(USE_DEVCONSOLE) || CONFIG_SIM_UART_NUMBER > 0
+#if defined(USE_DEVCONSOLE) || defined(CONFIG_SIM_UART)
/****************************************************************************
* Pre-processor Definitions
****************************************************************************/
-#define BUFSIZE 256
+#ifdef CONFIG_SIM_UART_BUFFER_SIZE
Review comment:
@fjpanag do you want to apply the suggested 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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] fjpanag commented on a change in pull request #5295: sim: Added Kconfig option for UART buffer size.
Posted by GitBox <gi...@apache.org>.
fjpanag commented on a change in pull request #5295:
URL: https://github.com/apache/incubator-nuttx/pull/5295#discussion_r793467169
##########
File path: arch/sim/src/sim/up_uart.c
##########
@@ -31,12 +31,16 @@
#include "up_internal.h"
-#if defined(USE_DEVCONSOLE) || CONFIG_SIM_UART_NUMBER > 0
+#if defined(USE_DEVCONSOLE) || defined(CONFIG_SIM_UART)
/****************************************************************************
* Pre-processor Definitions
****************************************************************************/
-#define BUFSIZE 256
+#ifdef CONFIG_SIM_UART_BUFFER_SIZE
Review comment:
I'm pushing this change today.
My only concern was that defining a `CONFIG_*` macro in a place other than a Kconfig may be confusing, with no real benefit.
Nevertheless, I will proceed as suggested.
--
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-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org