You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mynewt.apache.org by GitBox <gi...@apache.org> on 2020/12/07 15:37:09 UTC
[GitHub] [mynewt-core] kasjer opened a new pull request #2430: standardize OS_TICKS_PER_SEC definition
kasjer opened a new pull request #2430:
URL: https://github.com/apache/mynewt-core/pull/2430
In several instances OS_TICKS_PER_SEC was configurable via syscfg.
This PR proposes to have same behavior across all MCUs.
Now only one definition in os_time.h exists and it gets it form syscfg.
All MCUs have definition moved to syscfg.
In few cases other syscfg values that referred to OS_TICKS_PER_SEC
are using MYNEWT_VAL(OS_TICKS_PER_SEC) to avoid circular
dependency problem.
----------------------------------------------------------------
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] [mynewt-core] apache-mynewt-bot commented on pull request #2430: standardize OS_TICKS_PER_SEC definition
Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot commented on pull request #2430:
URL: https://github.com/apache/mynewt-core/pull/2430#issuecomment-756763545
<!-- style-bot -->
## Style check summary
#### No suggestions at this time!
----------------------------------------------------------------
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] [mynewt-core] kasjer merged pull request #2430: standardize OS_TICKS_PER_SEC definition
Posted by GitBox <gi...@apache.org>.
kasjer merged pull request #2430:
URL: https://github.com/apache/mynewt-core/pull/2430
----------------------------------------------------------------
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] [mynewt-core] kasjer commented on pull request #2430: standardize OS_TICKS_PER_SEC definition
Posted by GitBox <gi...@apache.org>.
kasjer commented on pull request #2430:
URL: https://github.com/apache/mynewt-core/pull/2430#issuecomment-756763763
re-base was required over already approved version due to conflicts in pic32 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] [mynewt-core] apache-mynewt-bot removed a comment on pull request #2430: standardize OS_TICKS_PER_SEC definition
Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot removed a comment on pull request #2430:
URL: https://github.com/apache/mynewt-core/pull/2430#issuecomment-747400121
<!-- style-bot -->
## Style check summary
#### No suggestions at this time!
----------------------------------------------------------------
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] [mynewt-core] apache-mynewt-bot commented on pull request #2430: standardize OS_TICKS_PER_SEC definition
Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot commented on pull request #2430:
URL: https://github.com/apache/mynewt-core/pull/2430#issuecomment-739996011
<!-- style-bot -->
## Style check summary
#### No suggestions at this time!
----------------------------------------------------------------
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] [mynewt-core] caspermeijn commented on a change in pull request #2430: standardize OS_TICKS_PER_SEC definition
Posted by GitBox <gi...@apache.org>.
caspermeijn commented on a change in pull request #2430:
URL: https://github.com/apache/mynewt-core/pull/2430#discussion_r539636969
##########
File path: net/ip/native_sockets/syscfg.yml
##########
@@ -27,7 +27,7 @@ syscfg.defs:
description: >
The frequency at which to poll for received data. Units
are OS ticks.
- value: 'OS_TICKS_PER_SEC / 5'
+ value: 'MYNEWT_VAL(OS_TICKS_PER_SEC) / 5'
Review comment:
The `MYNEWT_VAL` macro is not needed, right? As `os_time.h` defines `OS_TICKS_PER_SEC`. So as long as all users of `NATIVE_SOCKETS_POLL_ITVL` include `os_time.h` it would work.
Or is the problem that you need a `MYNEWT_VAL` to evaluate the `MYNEWT_VAL`? Because that would still be needed.
----------------------------------------------------------------
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] [mynewt-core] caspermeijn commented on pull request #2430: standardize OS_TICKS_PER_SEC definition
Posted by GitBox <gi...@apache.org>.
caspermeijn commented on pull request #2430:
URL: https://github.com/apache/mynewt-core/pull/2430#issuecomment-742057032
I can't compile with this version of mynewt-core:
```
$ newt build klok-pinetime
Building target targets/klok-pinetime
Error: Priority violations detected (Packages can only override settings defined by packages of lower priority):
Package: @apache-mynewt-core/hw/mcu/nordic/nrf52xxx overriding setting: OS_TICKS_PER_SEC defined by @apache-mynewt-core/kernel/os
Setting history (newest -> oldest):
OS_TICKS_PER_SEC: [@apache-mynewt-core/hw/mcu/nordic/nrf52xxx:128, @apache-mynewt-core/kernel/os:]
```
----------------------------------------------------------------
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] [mynewt-core] apache-mynewt-bot removed a comment on pull request #2430: standardize OS_TICKS_PER_SEC definition
Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot removed a comment on pull request #2430:
URL: https://github.com/apache/mynewt-core/pull/2430#issuecomment-747400121
<!-- style-bot -->
## Style check summary
#### No suggestions at this time!
----------------------------------------------------------------
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] [mynewt-core] apache-mynewt-bot commented on pull request #2430: standardize OS_TICKS_PER_SEC definition
Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot commented on pull request #2430:
URL: https://github.com/apache/mynewt-core/pull/2430#issuecomment-747400121
<!-- style-bot -->
## Style check summary
#### No suggestions at this time!
----------------------------------------------------------------
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] [mynewt-core] caspermeijn commented on pull request #2430: standardize OS_TICKS_PER_SEC definition
Posted by GitBox <gi...@apache.org>.
caspermeijn commented on pull request #2430:
URL: https://github.com/apache/mynewt-core/pull/2430#issuecomment-742265538
@andrzej-kaczmarek Thanks, with latest newt it builds.
@wes3 So basically both question are for to get more accurate time handling. Got it.
@kasjer `hw/util/button` build correctly for me with these changes.
----------------------------------------------------------------
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] [mynewt-core] apache-mynewt-bot removed a comment on pull request #2430: standardize OS_TICKS_PER_SEC definition
Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot removed a comment on pull request #2430:
URL: https://github.com/apache/mynewt-core/pull/2430#issuecomment-741802404
<!-- style-bot -->
## Style check summary
#### No suggestions at this time!
----------------------------------------------------------------
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] [mynewt-core] andrzej-kaczmarek commented on pull request #2430: standardize OS_TICKS_PER_SEC definition
Posted by GitBox <gi...@apache.org>.
andrzej-kaczmarek commented on pull request #2430:
URL: https://github.com/apache/mynewt-core/pull/2430#issuecomment-742069559
@caspermeijn you need to use latest newt (master) to build
----------------------------------------------------------------
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] [mynewt-core] kasjer commented on a change in pull request #2430: standardize OS_TICKS_PER_SEC definition
Posted by GitBox <gi...@apache.org>.
kasjer commented on a change in pull request #2430:
URL: https://github.com/apache/mynewt-core/pull/2430#discussion_r539434707
##########
File path: net/ip/native_sockets/syscfg.yml
##########
@@ -27,7 +27,7 @@ syscfg.defs:
description: >
The frequency at which to poll for received data. Units
are OS ticks.
- value: 'OS_TICKS_PER_SEC / 5'
+ value: 'MYNEWT_VAL(OS_TICKS_PER_SEC) / 5'
Review comment:
It turns out that such expressions are not handled sensible by newt tool.
----------------------------------------------------------------
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] [mynewt-core] apache-mynewt-bot removed a comment on pull request #2430: standardize OS_TICKS_PER_SEC definition
Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot removed a comment on pull request #2430:
URL: https://github.com/apache/mynewt-core/pull/2430#issuecomment-739996011
<!-- style-bot -->
## Style check summary
#### No suggestions at this time!
----------------------------------------------------------------
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] [mynewt-core] apache-mynewt-bot commented on pull request #2430: standardize OS_TICKS_PER_SEC definition
Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot commented on pull request #2430:
URL: https://github.com/apache/mynewt-core/pull/2430#issuecomment-741802404
<!-- style-bot -->
## Style check summary
#### No suggestions at this time!
----------------------------------------------------------------
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] [mynewt-core] kasjer commented on pull request #2430: standardize OS_TICKS_PER_SEC definition
Posted by GitBox <gi...@apache.org>.
kasjer commented on pull request #2430:
URL: https://github.com/apache/mynewt-core/pull/2430#issuecomment-756763763
re-base was required over already approved version due to conflicts in pic32 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] [mynewt-core] wes3 edited a comment on pull request #2430: standardize OS_TICKS_PER_SEC definition
Posted by GitBox <gi...@apache.org>.
wes3 edited a comment on pull request #2430:
URL: https://github.com/apache/mynewt-core/pull/2430#issuecomment-742192110
@caspermeijn I can answer some of the questions you posed.
1) Why would someone want to change this value?
Some may want to change this value so that the time resolution for the os tick to be greater or less than the default value set. Some folks may want a 1 msec OS tick resolution whereas some may want longer. If you use 128 ticks per second the rate is, I believe 7.8125 msecs per tick. That may be too long for some applications (for example).
2) Why do all mcu define their own value?
The main reason for this, I believe, is the default timer used for generating the os tick. We wanted to be able to generate the tick period exactly. Consider a 1 MHz timer and a 32768 Hz timer. For the 32768 timer, generating a 10msec os tick is not easy given integer number of ticks. That is why 128 is chosen for those MCUs as the 7.8125msec period is an integer number of ticks for a 32768 timer. For 1MHz, a 10msec resolution was chosen since that is an integer number of ticks for that timer.
3) Would it make sense to standardize the actual value?
I think, given the reason all mcus define their own values, no. There are really only two values I think used that are "standard": 128 and 100. So this seems ok to 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] [mynewt-core] apache-mynewt-bot commented on pull request #2430: standardize OS_TICKS_PER_SEC definition
Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot commented on pull request #2430:
URL: https://github.com/apache/mynewt-core/pull/2430#issuecomment-756763545
<!-- style-bot -->
## Style check summary
#### No suggestions at this time!
----------------------------------------------------------------
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] [mynewt-core] wes3 commented on pull request #2430: standardize OS_TICKS_PER_SEC definition
Posted by GitBox <gi...@apache.org>.
wes3 commented on pull request #2430:
URL: https://github.com/apache/mynewt-core/pull/2430#issuecomment-742192110
@caspermeijn I can answer some of the questions you posed.
1) Why would someone want to change this value?
Some may want to change this value so that the time resolution for the os tick to be greater or less than the default value set. Some folks may want a 1 msec OS tick resolution whereas some may want longer. If you use 128 ticks per second the rate is, I believe 7.8125 msecs per tick.
2) Why do all mcu define their own value?
The main reason for this, I believe, is the default timer used for generating the os tick. We wanted to be able to generate the tick period exactly. Consider a 1 MHz timer and a 32768 Hz timer. For the 32768 timer, generating a 10msec os tick is not easy given integer number of ticks. That is why 128 is chosen for those MCUs as the 7.8125msec period is an integer number of ticks for a 32768 timer. For 1MHz, a 10msec resolution was chosen since that is an integer number of ticks for that timer.
3) Would it make sense to standardize the actual value?
I think, given the reason all mcus define their own values, no. There are really only two values I think used that are "standard": 128 and 100. So this seems ok to 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