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/10/26 06:08:17 UTC
[GitHub] [incubator-nuttx] sebastianene07 opened a new pull request #2116: arch/sim: Enable SIM_WALLTIME option by default
sebastianene07 opened a new pull request #2116:
URL: https://github.com/apache/incubator-nuttx/pull/2116
### Summary of Changes ###
Running the NuttX simulation 'as fast as possible' breaks the features
that depend on timing: eg. the Bluetooth stack. Enabling this option by
default SIM_WALLTIME=y will introduce delays and will tick the simulation at
a real pace.
## Impact
## Testing
Signed-off-by: Sebastian Ene <se...@apache.org>
----------------------------------------------------------------
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] xiaoxiang781216 commented on a change in pull request #2116: arch/sim: Enable SIM_WALLTIME option by default
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #2116:
URL: https://github.com/apache/incubator-nuttx/pull/2116#discussion_r512005289
##########
File path: arch/sim/Kconfig
##########
@@ -225,8 +224,7 @@ if SIM_FRAMEBUFFER
config SIM_X11FB
bool "Use X11 window"
default n
- select SCHED_LPWORK
- select SIM_WALLTIME
+ depends on (SCHED_LPWORK && SIM_WALLTIME)
Review comment:
need keep SCHED_LPWORK as select too
----------------------------------------------------------------
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] v01d commented on a change in pull request #2116: arch/sim: Enable SIM_WALLTIME option by default
Posted by GitBox <gi...@apache.org>.
v01d commented on a change in pull request #2116:
URL: https://github.com/apache/incubator-nuttx/pull/2116#discussion_r511931050
##########
File path: arch/sim/Kconfig
##########
@@ -99,7 +99,7 @@ endchoice
config SIM_WALLTIME
bool "Run the simulation at a fixed cadence in near real-time"
- default n
+ default y
Review comment:
If the default is `y`, I think it is OK to have it depend on it. When a default is `n`, the depend makes it hard to discover the option, in that case `select` is more useful.
----------------------------------------------------------------
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] sebastianene07 commented on a change in pull request #2116: arch/sim: Enable SIM_WALLTIME option by default
Posted by GitBox <gi...@apache.org>.
sebastianene07 commented on a change in pull request #2116:
URL: https://github.com/apache/incubator-nuttx/pull/2116#discussion_r512060384
##########
File path: arch/sim/Kconfig
##########
@@ -225,8 +224,7 @@ if SIM_FRAMEBUFFER
config SIM_X11FB
bool "Use X11 window"
default n
- select SCHED_LPWORK
- select SIM_WALLTIME
+ depends on (SCHED_LPWORK && SIM_WALLTIME)
Review comment:
oh, 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] sebastianene07 commented on a change in pull request #2116: arch/sim: Enable SIM_WALLTIME option by default
Posted by GitBox <gi...@apache.org>.
sebastianene07 commented on a change in pull request #2116:
URL: https://github.com/apache/incubator-nuttx/pull/2116#discussion_r511781196
##########
File path: arch/sim/Kconfig
##########
@@ -99,7 +99,7 @@ endchoice
config SIM_WALLTIME
bool "Run the simulation at a fixed cadence in near real-time"
- default n
+ default y
Review comment:
I will edit the Kconfig and add a depends on rule for SIM_HCISOCKET.
----------------------------------------------------------------
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] v01d commented on a change in pull request #2116: arch/sim: Enable SIM_WALLTIME option by default
Posted by GitBox <gi...@apache.org>.
v01d commented on a change in pull request #2116:
URL: https://github.com/apache/incubator-nuttx/pull/2116#discussion_r511942040
##########
File path: arch/sim/Kconfig
##########
@@ -99,7 +99,7 @@ endchoice
config SIM_WALLTIME
bool "Run the simulation at a fixed cadence in near real-time"
- default n
+ default y
Review comment:
I'd imagine it was due to WALLTIME being `n` by default. In that case I agree that those should also be changed into `depends`
----------------------------------------------------------------
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] xiaoxiang781216 commented on a change in pull request #2116: arch/sim: Enable SIM_WALLTIME option by default
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #2116:
URL: https://github.com/apache/incubator-nuttx/pull/2116#discussion_r511914282
##########
File path: arch/sim/Kconfig
##########
@@ -99,7 +99,7 @@ endchoice
config SIM_WALLTIME
bool "Run the simulation at a fixed cadence in near real-time"
- default n
+ default y
Review comment:
But other place use select, should we unify the usage?
----------------------------------------------------------------
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] sebastianene07 commented on a change in pull request #2116: arch/sim: Enable SIM_WALLTIME option by default
Posted by GitBox <gi...@apache.org>.
sebastianene07 commented on a change in pull request #2116:
URL: https://github.com/apache/incubator-nuttx/pull/2116#discussion_r511963043
##########
File path: arch/sim/Kconfig
##########
@@ -99,7 +99,7 @@ endchoice
config SIM_WALLTIME
bool "Run the simulation at a fixed cadence in near real-time"
- default n
+ default y
Review comment:
I changed these options to depend on ```SIM_WALLTIME```
----------------------------------------------------------------
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] btashton commented on pull request #2116: arch/sim: Enable SIM_WALLTIME option by default
Posted by GitBox <gi...@apache.org>.
btashton commented on pull request #2116:
URL: https://github.com/apache/incubator-nuttx/pull/2116#issuecomment-716330717
You are going to need to refresh these configs:
```
sim/sim/sim/configs/minibasic/defconfig:CONFIG_SIM_WALLTIME=y
sim/sim/sim/configs/unionfs/defconfig:CONFIG_SIM_WALLTIME=y
sim/sim/sim/configs/smp/defconfig:CONFIG_SIM_WALLTIME=y
sim/sim/sim/configs/loadable/defconfig:CONFIG_SIM_WALLTIME=y
sim/sim/sim/configs/bthcisock/defconfig:CONFIG_SIM_WALLTIME=y
sim/sim/sim/configs/tcploop/defconfig:CONFIG_SIM_WALLTIME=y
sim/sim/sim/configs/rpproxy/defconfig:CONFIG_SIM_WALLTIME=y
sim/sim/sim/configs/duktape/defconfig:CONFIG_SIM_WALLTIME=y
sim/sim/sim/configs/ipforward/defconfig:CONFIG_SIM_WALLTIME=y
sim/sim/sim/configs/nshcromfs/defconfig:CONFIG_SIM_WALLTIME=y
sim/sim/sim/configs/nsh/defconfig:CONFIG_SIM_WALLTIME=y
sim/sim/sim/configs/bas/defconfig:CONFIG_SIM_WALLTIME=y
```
----------------------------------------------------------------
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] xiaoxiang781216 commented on a change in pull request #2116: arch/sim: Enable SIM_WALLTIME option by default
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #2116:
URL: https://github.com/apache/incubator-nuttx/pull/2116#discussion_r511941320
##########
File path: arch/sim/Kconfig
##########
@@ -99,7 +99,7 @@ endchoice
config SIM_WALLTIME
bool "Run the simulation at a fixed cadence in near real-time"
- default n
+ default y
Review comment:
@v01d Either select or depend is fine for me, but we need ensure the usage consistence: it's very strange that SIM_HCISOCKET depend on SIM_WALLTIME but SIM_NETDEV and SIM_X11FB select SIM_WALLTIME in the same Kconfig.
----------------------------------------------------------------
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] sebastianene07 commented on pull request #2116: arch/sim: Enable SIM_WALLTIME option by default
Posted by GitBox <gi...@apache.org>.
sebastianene07 commented on pull request #2116:
URL: https://github.com/apache/incubator-nuttx/pull/2116#issuecomment-716347475
> You are going to need to refresh these configs:
>
> ```
> sim/sim/sim/configs/minibasic/defconfig:CONFIG_SIM_WALLTIME=y
> sim/sim/sim/configs/unionfs/defconfig:CONFIG_SIM_WALLTIME=y
> sim/sim/sim/configs/smp/defconfig:CONFIG_SIM_WALLTIME=y
> sim/sim/sim/configs/loadable/defconfig:CONFIG_SIM_WALLTIME=y
> sim/sim/sim/configs/bthcisock/defconfig:CONFIG_SIM_WALLTIME=y
> sim/sim/sim/configs/tcploop/defconfig:CONFIG_SIM_WALLTIME=y
> sim/sim/sim/configs/rpproxy/defconfig:CONFIG_SIM_WALLTIME=y
> sim/sim/sim/configs/duktape/defconfig:CONFIG_SIM_WALLTIME=y
> sim/sim/sim/configs/ipforward/defconfig:CONFIG_SIM_WALLTIME=y
> sim/sim/sim/configs/nshcromfs/defconfig:CONFIG_SIM_WALLTIME=y
> sim/sim/sim/configs/nsh/defconfig:CONFIG_SIM_WALLTIME=y
> sim/sim/sim/configs/bas/defconfig:CONFIG_SIM_WALLTIME=y
> ```
I refreshed these configs.
----------------------------------------------------------------
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] v01d commented on a change in pull request #2116: arch/sim: Enable SIM_WALLTIME option by default
Posted by GitBox <gi...@apache.org>.
v01d commented on a change in pull request #2116:
URL: https://github.com/apache/incubator-nuttx/pull/2116#discussion_r511931853
##########
File path: arch/sim/Kconfig
##########
@@ -99,7 +99,7 @@ endchoice
config SIM_WALLTIME
bool "Run the simulation at a fixed cadence in near real-time"
- default n
+ default y
Review comment:
If you are OK with this @xiaoxiang781216 please merge.
----------------------------------------------------------------
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] xiaoxiang781216 merged pull request #2116: arch/sim: Enable SIM_WALLTIME option by default
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged pull request #2116:
URL: https://github.com/apache/incubator-nuttx/pull/2116
----------------------------------------------------------------
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] xiaoxiang781216 commented on a change in pull request #2116: arch/sim: Enable SIM_WALLTIME option by default
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #2116:
URL: https://github.com/apache/incubator-nuttx/pull/2116#discussion_r511968647
##########
File path: arch/sim/Kconfig
##########
@@ -134,10 +134,7 @@ endif
config SIM_NETDEV
bool "Simulated Network Device"
default y
- depends on NET_ETHERNET
- select ARCH_HAVE_NETDEV_STATISTICS
- select SCHED_LPWORK
- select SIM_WALLTIME
+ depends on (NET_ETHERNET && SIM_WALLTIME && ARCH_HAVE_NETDEV_STATISTICS && SCHED_LPWORK)
Review comment:
ARCH_HAVE_NETDEV_STATISTICS is designed to be selected.
----------------------------------------------------------------
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] xiaoxiang781216 commented on a change in pull request #2116: arch/sim: Enable SIM_WALLTIME option by default
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #2116:
URL: https://github.com/apache/incubator-nuttx/pull/2116#discussion_r511971833
##########
File path: arch/sim/Kconfig
##########
@@ -134,10 +134,7 @@ endif
config SIM_NETDEV
bool "Simulated Network Device"
default y
- depends on NET_ETHERNET
- select ARCH_HAVE_NETDEV_STATISTICS
- select SCHED_LPWORK
- select SIM_WALLTIME
+ depends on (NET_ETHERNET && SIM_WALLTIME && ARCH_HAVE_NETDEV_STATISTICS && SCHED_LPWORK)
Review comment:
ARCH_HAVE_NETDEV_STATISTICS is designed to be selected, and the most place select SCHED_LPWORK, not depend on SCHED_LPWORK. so it's better to change:
depends on NET_ETHERNET && SIM_WALLTIME
select ARCH_HAVE_NETDEV_STATISTICS
select SCHED_LPWORK
----------------------------------------------------------------
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] xiaoxiang781216 commented on a change in pull request #2116: arch/sim: Enable SIM_WALLTIME option by default
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #2116:
URL: https://github.com/apache/incubator-nuttx/pull/2116#discussion_r511969437
##########
File path: arch/sim/Kconfig
##########
@@ -134,10 +134,7 @@ endif
config SIM_NETDEV
bool "Simulated Network Device"
default y
- depends on NET_ETHERNET
- select ARCH_HAVE_NETDEV_STATISTICS
- select SCHED_LPWORK
- select SIM_WALLTIME
+ depends on (NET_ETHERNET && SIM_WALLTIME && ARCH_HAVE_NETDEV_STATISTICS && SCHED_LPWORK)
Review comment:
and the most place select SCHED_LPWORK not depend on SCHED_LPWORK
----------------------------------------------------------------
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] sebastianene07 commented on a change in pull request #2116: arch/sim: Enable SIM_WALLTIME option by default
Posted by GitBox <gi...@apache.org>.
sebastianene07 commented on a change in pull request #2116:
URL: https://github.com/apache/incubator-nuttx/pull/2116#discussion_r511986679
##########
File path: arch/sim/Kconfig
##########
@@ -134,10 +134,7 @@ endif
config SIM_NETDEV
bool "Simulated Network Device"
default y
- depends on NET_ETHERNET
- select ARCH_HAVE_NETDEV_STATISTICS
- select SCHED_LPWORK
- select SIM_WALLTIME
+ depends on (NET_ETHERNET && SIM_WALLTIME && ARCH_HAVE_NETDEV_STATISTICS && SCHED_LPWORK)
Review comment:
Ok I updated to this
----------------------------------------------------------------
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] xiaoxiang781216 commented on a change in pull request #2116: arch/sim: Enable SIM_WALLTIME option by default
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #2116:
URL: https://github.com/apache/incubator-nuttx/pull/2116#discussion_r511743209
##########
File path: arch/sim/Kconfig
##########
@@ -99,7 +99,7 @@ endchoice
config SIM_WALLTIME
bool "Run the simulation at a fixed cadence in near real-time"
- default n
+ default y
Review comment:
even we change SIM_WALLTIME default to y, we still need to select SIM_WALLTIME for SIM_HCISOCKET. Otherwise the user can generate the wrong combination by:
CONFIG_SIM_WALLTIME=n
CONFIG_SIM_HCISOCKET=y
That's why I think the default value change can't give any real help 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