[GitHub] [incubator-nuttx] yamt commented on a change in pull request #274: Improve sim timer
yamt commented on a change in pull request #274: Improve sim timer
URL: https://github.com/apache/incubator-nuttx/pull/274#discussion_r379908746
##
File path: arch/sim/src/sim/up_rtc.c
##
@@ -0,0 +1,127 @@
+/
+ * arch/sim/src/sim/up_rtc.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ /
+
+/
+ * Included Files
+ /
+
+#include
+
+#include
+#include
+#include
+
+#include "up_internal.h"
+
+/
+ * Private Function Prototypes
+ /
+
+static int sim_rtc_rdtime(FAR struct rtc_lowerhalf_s *lower,
+ FAR struct rtc_time *rtctime);
+static int sim_rtc_settime(FAR struct rtc_lowerhalf_s *lower,
+ FAR const struct rtc_time *rtctime);
+static bool sim_rtc_havesettime(FAR struct rtc_lowerhalf_s *lower);
+
+/
+ * Private Data
+ /
+
+static const struct rtc_ops_s g_sim_rtc_ops =
+{
+ .rdtime = sim_rtc_rdtime,
+ .settime = sim_rtc_settime,
+ .havesettime = sim_rtc_havesettime,
+};
+
+static struct rtc_lowerhalf_s g_sim_rtc =
+{
+ .ops = _sim_rtc_ops,
+};
+
+/
+ * Private Functions
+ /
+
+static int sim_rtc_rdtime(FAR struct rtc_lowerhalf_s *lower,
+ FAR struct rtc_time *rtctime)
+{
+ uint64_t nsec;
+ time_t sec;
+
+
+ nsec = host_gettime(true);
+ sec = nsec / NSEC_PER_SEC;
+ nsec -= sec * NSEC_PER_SEC;
+
+ gmtime_r(, (FAR struct tm *)rtctime);
+ rtctime->tm_nsec = nsec;
+
+ return OK;
+}
+
+static int sim_rtc_settime(FAR struct rtc_lowerhalf_s *lower,
+ FAR const struct rtc_time *rtctime)
+{
+ uint64_t nsec;
+
+ nsec = mktime((FAR struct tm *)rtctime);
+ nsec *= NSEC_PER_SEC;
+ nsec += rtctime->tm_nsec;
+ host_settime(true, nsec);
Review comment:
i'm fine to work on a patch by myself.
but i want to understand why @xiaoxiang781216 wants this behavior.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] yamt commented on a change in pull request #274: Improve sim timer
yamt commented on a change in pull request #274: Improve sim timer
URL: https://github.com/apache/incubator-nuttx/pull/274#discussion_r379905077
##
File path: arch/sim/src/sim/up_rtc.c
##
@@ -0,0 +1,127 @@
+/
+ * arch/sim/src/sim/up_rtc.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ /
+
+/
+ * Included Files
+ /
+
+#include
+
+#include
+#include
+#include
+
+#include "up_internal.h"
+
+/
+ * Private Function Prototypes
+ /
+
+static int sim_rtc_rdtime(FAR struct rtc_lowerhalf_s *lower,
+ FAR struct rtc_time *rtctime);
+static int sim_rtc_settime(FAR struct rtc_lowerhalf_s *lower,
+ FAR const struct rtc_time *rtctime);
+static bool sim_rtc_havesettime(FAR struct rtc_lowerhalf_s *lower);
+
+/
+ * Private Data
+ /
+
+static const struct rtc_ops_s g_sim_rtc_ops =
+{
+ .rdtime = sim_rtc_rdtime,
+ .settime = sim_rtc_settime,
+ .havesettime = sim_rtc_havesettime,
+};
+
+static struct rtc_lowerhalf_s g_sim_rtc =
+{
+ .ops = _sim_rtc_ops,
+};
+
+/
+ * Private Functions
+ /
+
+static int sim_rtc_rdtime(FAR struct rtc_lowerhalf_s *lower,
+ FAR struct rtc_time *rtctime)
+{
+ uint64_t nsec;
+ time_t sec;
+
+
+ nsec = host_gettime(true);
+ sec = nsec / NSEC_PER_SEC;
+ nsec -= sec * NSEC_PER_SEC;
+
+ gmtime_r(, (FAR struct tm *)rtctime);
+ rtctime->tm_nsec = nsec;
+
+ return OK;
+}
+
+static int sim_rtc_settime(FAR struct rtc_lowerhalf_s *lower,
+ FAR const struct rtc_time *rtctime)
+{
+ uint64_t nsec;
+
+ nsec = mktime((FAR struct tm *)rtctime);
+ nsec *= NSEC_PER_SEC;
+ nsec += rtctime->tm_nsec;
+ host_settime(true, nsec);
Review comment:
i don't understand why you need to adjust host os time to test nuttx ntp
client logic.
i'm not sure about your host os. but as far as i know it's common for OSes
not to prevent clock_settime from adjusting time even when network time sync
stuff is enabled. at least macOS is in this category.
IMO it's enough reason not to make the sim call clock_settime.
wrt user experience, I, as a user, would be very surprised if modifying
nuttx time in the sim modifies host OS time. i might be a minority though.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] yamt commented on a change in pull request #274: Improve sim timer
yamt commented on a change in pull request #274: Improve sim timer
URL: https://github.com/apache/incubator-nuttx/pull/274#discussion_r379833656
##
File path: arch/sim/src/sim/up_rtc.c
##
@@ -0,0 +1,127 @@
+/
+ * arch/sim/src/sim/up_rtc.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ /
+
+/
+ * Included Files
+ /
+
+#include
+
+#include
+#include
+#include
+
+#include "up_internal.h"
+
+/
+ * Private Function Prototypes
+ /
+
+static int sim_rtc_rdtime(FAR struct rtc_lowerhalf_s *lower,
+ FAR struct rtc_time *rtctime);
+static int sim_rtc_settime(FAR struct rtc_lowerhalf_s *lower,
+ FAR const struct rtc_time *rtctime);
+static bool sim_rtc_havesettime(FAR struct rtc_lowerhalf_s *lower);
+
+/
+ * Private Data
+ /
+
+static const struct rtc_ops_s g_sim_rtc_ops =
+{
+ .rdtime = sim_rtc_rdtime,
+ .settime = sim_rtc_settime,
+ .havesettime = sim_rtc_havesettime,
+};
+
+static struct rtc_lowerhalf_s g_sim_rtc =
+{
+ .ops = _sim_rtc_ops,
+};
+
+/
+ * Private Functions
+ /
+
+static int sim_rtc_rdtime(FAR struct rtc_lowerhalf_s *lower,
+ FAR struct rtc_time *rtctime)
+{
+ uint64_t nsec;
+ time_t sec;
+
+
+ nsec = host_gettime(true);
+ sec = nsec / NSEC_PER_SEC;
+ nsec -= sec * NSEC_PER_SEC;
+
+ gmtime_r(, (FAR struct tm *)rtctime);
+ rtctime->tm_nsec = nsec;
+
+ return OK;
+}
+
+static int sim_rtc_settime(FAR struct rtc_lowerhalf_s *lower,
+ FAR const struct rtc_time *rtctime)
+{
+ uint64_t nsec;
+
+ nsec = mktime((FAR struct tm *)rtctime);
+ nsec *= NSEC_PER_SEC;
+ nsec += rtctime->tm_nsec;
+ host_settime(true, nsec);
Review comment:
while adjustnig time with ntp server is common,
* it usually does not prevent clock_settime
* it isn't always possible. eg. offline machines
i don't understand your use case to modify host os's clock. can you explain
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] yamt commented on a change in pull request #274: Improve sim timer
yamt commented on a change in pull request #274: Improve sim timer
URL: https://github.com/apache/incubator-nuttx/pull/274#discussion_r379449390
##
File path: arch/sim/src/sim/up_rtc.c
##
@@ -0,0 +1,127 @@
+/
+ * arch/sim/src/sim/up_rtc.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ /
+
+/
+ * Included Files
+ /
+
+#include
+
+#include
+#include
+#include
+
+#include "up_internal.h"
+
+/
+ * Private Function Prototypes
+ /
+
+static int sim_rtc_rdtime(FAR struct rtc_lowerhalf_s *lower,
+ FAR struct rtc_time *rtctime);
+static int sim_rtc_settime(FAR struct rtc_lowerhalf_s *lower,
+ FAR const struct rtc_time *rtctime);
+static bool sim_rtc_havesettime(FAR struct rtc_lowerhalf_s *lower);
+
+/
+ * Private Data
+ /
+
+static const struct rtc_ops_s g_sim_rtc_ops =
+{
+ .rdtime = sim_rtc_rdtime,
+ .settime = sim_rtc_settime,
+ .havesettime = sim_rtc_havesettime,
+};
+
+static struct rtc_lowerhalf_s g_sim_rtc =
+{
+ .ops = _sim_rtc_ops,
+};
+
+/
+ * Private Functions
+ /
+
+static int sim_rtc_rdtime(FAR struct rtc_lowerhalf_s *lower,
+ FAR struct rtc_time *rtctime)
+{
+ uint64_t nsec;
+ time_t sec;
+
+
+ nsec = host_gettime(true);
+ sec = nsec / NSEC_PER_SEC;
+ nsec -= sec * NSEC_PER_SEC;
+
+ gmtime_r(, (FAR struct tm *)rtctime);
+ rtctime->tm_nsec = nsec;
+
+ return OK;
+}
+
+static int sim_rtc_settime(FAR struct rtc_lowerhalf_s *lower,
+ FAR const struct rtc_time *rtctime)
+{
+ uint64_t nsec;
+
+ nsec = mktime((FAR struct tm *)rtctime);
+ nsec *= NSEC_PER_SEC;
+ nsec += rtctime->tm_nsec;
+ host_settime(true, nsec);
Review comment:
i'm not sure what "the sync option" is. but i guess it isn't available for
every host environments, 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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] yamt commented on a change in pull request #274: Improve sim timer
yamt commented on a change in pull request #274: Improve sim timer
URL: https://github.com/apache/incubator-nuttx/pull/274#discussion_r379333291
##
File path: arch/sim/src/sim/up_rtc.c
##
@@ -0,0 +1,127 @@
+/
+ * arch/sim/src/sim/up_rtc.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ /
+
+/
+ * Included Files
+ /
+
+#include
+
+#include
+#include
+#include
+
+#include "up_internal.h"
+
+/
+ * Private Function Prototypes
+ /
+
+static int sim_rtc_rdtime(FAR struct rtc_lowerhalf_s *lower,
+ FAR struct rtc_time *rtctime);
+static int sim_rtc_settime(FAR struct rtc_lowerhalf_s *lower,
+ FAR const struct rtc_time *rtctime);
+static bool sim_rtc_havesettime(FAR struct rtc_lowerhalf_s *lower);
+
+/
+ * Private Data
+ /
+
+static const struct rtc_ops_s g_sim_rtc_ops =
+{
+ .rdtime = sim_rtc_rdtime,
+ .settime = sim_rtc_settime,
+ .havesettime = sim_rtc_havesettime,
+};
+
+static struct rtc_lowerhalf_s g_sim_rtc =
+{
+ .ops = _sim_rtc_ops,
+};
+
+/
+ * Private Functions
+ /
+
+static int sim_rtc_rdtime(FAR struct rtc_lowerhalf_s *lower,
+ FAR struct rtc_time *rtctime)
+{
+ uint64_t nsec;
+ time_t sec;
+
+
+ nsec = host_gettime(true);
+ sec = nsec / NSEC_PER_SEC;
+ nsec -= sec * NSEC_PER_SEC;
+
+ gmtime_r(, (FAR struct tm *)rtctime);
+ rtctime->tm_nsec = nsec;
+
+ return OK;
+}
+
+static int sim_rtc_settime(FAR struct rtc_lowerhalf_s *lower,
+ FAR const struct rtc_time *rtctime)
+{
+ uint64_t nsec;
+
+ nsec = mktime((FAR struct tm *)rtctime);
+ nsec *= NSEC_PER_SEC;
+ nsec += rtctime->tm_nsec;
+ host_settime(true, nsec);
Review comment:
i think it's better to keep a diff from host os clock, rather than changing
the host clock. how do you think?
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] yamt commented on a change in pull request #274: Improve sim timer
yamt commented on a change in pull request #274: Improve sim timer
URL: https://github.com/apache/incubator-nuttx/pull/274#discussion_r379289488
##
File path: arch/sim/src/sim/up_rtc.c
##
@@ -0,0 +1,127 @@
+/
+ * arch/sim/src/sim/up_rtc.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ /
+
+/
+ * Included Files
+ /
+
+#include
+
+#include
+#include
+#include
+
+#include "up_internal.h"
+
+/
+ * Private Function Prototypes
+ /
+
+static int sim_rtc_rdtime(FAR struct rtc_lowerhalf_s *lower,
+ FAR struct rtc_time *rtctime);
+static int sim_rtc_settime(FAR struct rtc_lowerhalf_s *lower,
+ FAR const struct rtc_time *rtctime);
+static bool sim_rtc_havesettime(FAR struct rtc_lowerhalf_s *lower);
+
+/
+ * Private Data
+ /
+
+static const struct rtc_ops_s g_sim_rtc_ops =
+{
+ .rdtime = sim_rtc_rdtime,
+ .settime = sim_rtc_settime,
+ .havesettime = sim_rtc_havesettime,
+};
+
+static struct rtc_lowerhalf_s g_sim_rtc =
+{
+ .ops = _sim_rtc_ops,
+};
+
+/
+ * Private Functions
+ /
+
+static int sim_rtc_rdtime(FAR struct rtc_lowerhalf_s *lower,
+ FAR struct rtc_time *rtctime)
+{
+ uint64_t nsec;
+ time_t sec;
+
+
+ nsec = host_gettime(true);
+ sec = nsec / NSEC_PER_SEC;
+ nsec -= sec * NSEC_PER_SEC;
+
+ gmtime_r(, (FAR struct tm *)rtctime);
+ rtctime->tm_nsec = nsec;
+
+ return OK;
+}
+
+static int sim_rtc_settime(FAR struct rtc_lowerhalf_s *lower,
+ FAR const struct rtc_time *rtctime)
+{
+ uint64_t nsec;
+
+ nsec = mktime((FAR struct tm *)rtctime);
+ nsec *= NSEC_PER_SEC;
+ nsec += rtctime->tm_nsec;
+ host_settime(true, nsec);
Review comment:
isn't host clock_settime failing for some reasons?
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] yamt commented on a change in pull request #274: Improve sim timer
yamt commented on a change in pull request #274: Improve sim timer
URL: https://github.com/apache/incubator-nuttx/pull/274#discussion_r379064231
##
File path: arch/sim/src/sim/up_rtc.c
##
@@ -0,0 +1,127 @@
+/
+ * arch/sim/src/sim/up_rtc.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ /
+
+/
+ * Included Files
+ /
+
+#include
+
+#include
+#include
+#include
+
+#include "up_internal.h"
+
+/
+ * Private Function Prototypes
+ /
+
+static int sim_rtc_rdtime(FAR struct rtc_lowerhalf_s *lower,
+ FAR struct rtc_time *rtctime);
+static int sim_rtc_settime(FAR struct rtc_lowerhalf_s *lower,
+ FAR const struct rtc_time *rtctime);
+static bool sim_rtc_havesettime(FAR struct rtc_lowerhalf_s *lower);
+
+/
+ * Private Data
+ /
+
+static const struct rtc_ops_s g_sim_rtc_ops =
+{
+ .rdtime = sim_rtc_rdtime,
+ .settime = sim_rtc_settime,
+ .havesettime = sim_rtc_havesettime,
+};
+
+static struct rtc_lowerhalf_s g_sim_rtc =
+{
+ .ops = _sim_rtc_ops,
+};
+
+/
+ * Private Functions
+ /
+
+static int sim_rtc_rdtime(FAR struct rtc_lowerhalf_s *lower,
+ FAR struct rtc_time *rtctime)
+{
+ uint64_t nsec;
+ time_t sec;
+
+
+ nsec = host_gettime(true);
+ sec = nsec / NSEC_PER_SEC;
+ nsec -= sec * NSEC_PER_SEC;
+
+ gmtime_r(, (FAR struct tm *)rtctime);
+ rtctime->tm_nsec = nsec;
+
+ return OK;
+}
+
+static int sim_rtc_settime(FAR struct rtc_lowerhalf_s *lower,
+ FAR const struct rtc_time *rtctime)
+{
+ uint64_t nsec;
+
+ nsec = mktime((FAR struct tm *)rtctime);
+ nsec *= NSEC_PER_SEC;
+ nsec += rtctime->tm_nsec;
+ host_settime(true, nsec);
Review comment:
does this mean the sim can modify the clock of the host os?
it's a surprising behavior 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:
us...@infra.apache.org
With regards,
Apache Git Services
