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/03/04 16:28:54 UTC
[GitHub] [mynewt-core] gonccalo opened a new pull request #2227: fix
incorrect check in lis2ds12_set_full_scale
gonccalo opened a new pull request #2227: fix incorrect check in lis2ds12_set_full_scale
URL: https://github.com/apache/mynewt-core/pull/2227
Checking if `fs` has a value bigger than `LIS2DS12_FS_16G` is incorrect because `LIS2DS12_FS_4G` and `LIS2DS12_FS_8G` have a bigger numeric value and are valid configurations for this device.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [mynewt-core] gonccalo edited a comment on issue #2227: fix
incorrect check in lis2ds12_set_full_scale
Posted by GitBox <gi...@apache.org>.
gonccalo edited a comment on issue #2227: fix incorrect check in lis2ds12_set_full_scale
URL: https://github.com/apache/mynewt-core/pull/2227#issuecomment-595345794
I also found some problems in the `lis2ds12_set_rate` function.
The `LIS2DS12_DATA_RATE_LP_10BIT_800HZ` is defined as 0x70 when it should be 0xF0 according to the documentation. The condition check for the rate argument is also not the best one (and is incorrect right now).
Should I open a new pull request or can I make these changes 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
With regards,
Apache Git Services
[GitHub] [mynewt-core] apache-mynewt-bot commented on issue #2227: fix
incorrect check in lis2ds12_set_full_scale
Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot commented on issue #2227: fix incorrect check in lis2ds12_set_full_scale
URL: https://github.com/apache/mynewt-core/pull/2227#issuecomment-594665191
<!-- 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
With regards,
Apache Git Services
[GitHub] [mynewt-core] kasjer commented on a change in pull request #2227:
fix incorrect check in lis2ds12_set_full_scale
Posted by GitBox <gi...@apache.org>.
kasjer commented on a change in pull request #2227: fix incorrect check in lis2ds12_set_full_scale
URL: https://github.com/apache/mynewt-core/pull/2227#discussion_r388205193
##########
File path: hw/drivers/sensors/lis2ds12/src/lis2ds12.c
##########
@@ -520,7 +520,7 @@ lis2ds12_set_full_scale(struct sensor_itf *itf, uint8_t fs)
int rc;
uint8_t reg;
- if (fs > LIS2DS12_FS_16G) {
+ if (fs > LIS2DS12_FS_8G) {
Review comment:
Code was wrong, thanks @gonccalo, because it rejected correct value.
This check implies that value could be wrong, but it could be wrong in more ways that this condition checks.
So condition that validates correctness of full scale argument should rather be:
if (0 != (fs & ~LIS2DS12_CTRL_REG1_FS))
Condition fs > LIS2DS12_FS_8G on the first look suggest that 8G is limit for this sensor.
So @gonccalo would you mind amending 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
With regards,
Apache Git Services
[GitHub] [mynewt-core] rymanluk commented on issue #2227: fix incorrect
check in lis2ds12_set_full_scale
Posted by GitBox <gi...@apache.org>.
rymanluk commented on issue #2227: fix incorrect check in lis2ds12_set_full_scale
URL: https://github.com/apache/mynewt-core/pull/2227#issuecomment-597492552
@gonccalo , we have noticed that something went wrong when squashing and merging your PR and your patch is in the tree but with incorrect author information. Sorry about that, we need to look closer into github to figure out what happen. Promise it will not happen again. Thanks!
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [mynewt-core] apache-mynewt-bot commented on issue #2227: fix
incorrect check in lis2ds12_set_full_scale
Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot commented on issue #2227: fix incorrect check in lis2ds12_set_full_scale
URL: https://github.com/apache/mynewt-core/pull/2227#issuecomment-595364965
<!-- 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
With regards,
Apache Git Services
[GitHub] [mynewt-core] gonccalo commented on issue #2227: fix incorrect
check in lis2ds12_set_full_scale
Posted by GitBox <gi...@apache.org>.
gonccalo commented on issue #2227: fix incorrect check in lis2ds12_set_full_scale
URL: https://github.com/apache/mynewt-core/pull/2227#issuecomment-595345794
I also found some problems in the `lis2ds12_set_rate` function.
The `LIS2DS12_DATA_RATE_LP_10BIT_800HZ` is defined as 0x70 when it should be 0xF0 according to the documentation. The condition check for the rate argument is also not the best one (and is incorrect right 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [mynewt-core] apache-mynewt-bot removed a comment on issue #2227:
fix incorrect check in lis2ds12_set_full_scale
Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot removed a comment on issue #2227: fix incorrect check in lis2ds12_set_full_scale
URL: https://github.com/apache/mynewt-core/pull/2227#issuecomment-595364965
<!-- 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
With regards,
Apache Git Services
[GitHub] [mynewt-core] vrahane commented on issue #2227: fix incorrect check
in lis2ds12_set_full_scale
Posted by GitBox <gi...@apache.org>.
vrahane commented on issue #2227: fix incorrect check in lis2ds12_set_full_scale
URL: https://github.com/apache/mynewt-core/pull/2227#issuecomment-597296871
@gonccalo, appreciate your changes and bug fix. Please keep up the good work, we need all the help we can get 🙂
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [mynewt-core] utzig commented on issue #2227: fix incorrect check
in lis2ds12_set_full_scale
Posted by GitBox <gi...@apache.org>.
utzig commented on issue #2227: fix incorrect check in lis2ds12_set_full_scale
URL: https://github.com/apache/mynewt-core/pull/2227#issuecomment-597524110
> @gonccalo , we have noticed that something went wrong when squashing and merging your PR and your patch is in the tree but with incorrect author information. Sorry about that, we need to look closer into github to figure out what happen. Promise it will not happen again. Thanks!
https://github.com/isaacs/github/issues/1368
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [mynewt-core] apache-mynewt-bot removed a comment on issue #2227:
fix incorrect check in lis2ds12_set_full_scale
Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot removed a comment on issue #2227: fix incorrect check in lis2ds12_set_full_scale
URL: https://github.com/apache/mynewt-core/pull/2227#issuecomment-594665191
<!-- 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
With regards,
Apache Git Services
[GitHub] [mynewt-core] vrahane merged pull request #2227: fix incorrect
check in lis2ds12_set_full_scale
Posted by GitBox <gi...@apache.org>.
vrahane merged pull request #2227: fix incorrect check in lis2ds12_set_full_scale
URL: https://github.com/apache/mynewt-core/pull/2227
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [mynewt-core] apache-mynewt-bot commented on issue #2227: fix
incorrect check in lis2ds12_set_full_scale
Posted by GitBox <gi...@apache.org>.
apache-mynewt-bot commented on issue #2227: fix incorrect check in lis2ds12_set_full_scale
URL: https://github.com/apache/mynewt-core/pull/2227#issuecomment-595390925
<!-- style-bot -->
## Style check summary
### Our coding style is [here!](https://github.com/apache/mynewt-core/blob/master/CODING_STANDARDS.md)
#### hw/drivers/sensors/lis2ds12/src/lis2ds12.c
<details>
```diff
@@ -584,7 +584,7 @@
int rc;
uint8_t reg;
- // TODO probably not the best check for me
+ /* TODO probably not the best check for me */
if (0 != (rate & ~(LIS2DS12_CTRL_REG1_ODR | LIS2DS12_CTRL_REG1_HF_ODR))) {
LIS2DS12_LOG_ERROR("Invalid rate value\n");
rc = SYS_EINVAL;
```
</details>
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [mynewt-core] utzig commented on issue #2227: fix incorrect check
in lis2ds12_set_full_scale
Posted by GitBox <gi...@apache.org>.
utzig commented on issue #2227: fix incorrect check in lis2ds12_set_full_scale
URL: https://github.com/apache/mynewt-core/pull/2227#issuecomment-597525674
I'd rather disable both "Squash and merge" and "Create a merge commit" options, one is a Github rather then a git concept, the other changes how the author organized his/her commits. Authors should be responsible for squashing, rebasing, and organizing commits.
----------------------------------------------------------------
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
With regards,
Apache Git Services