Skip to content

drivers/lps331ap: apply unified params definition scheme + cleanup#8757

Merged
dylad merged 4 commits intoRIOT-OS:new_i2c_iffrom
aabadie:pr/drivers/params/lps331ap
Jul 9, 2018
Merged

drivers/lps331ap: apply unified params definition scheme + cleanup#8757
dylad merged 4 commits intoRIOT-OS:new_i2c_iffrom
aabadie:pr/drivers/params/lps331ap

Conversation

@aabadie
Copy link
Copy Markdown
Contributor

@aabadie aabadie commented Mar 8, 2018 •
edited
Loading

Contribution description

This PR update the params definitions scheme for this device driver and does some cleanup in the internal implementation.

Also forgot this one last week.

Issues/PRs references

Initially done in #7937 and related to #7519

#6577

@aabadie aabadie added Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation Area: drivers Area: Device drivers CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Mar 8, 2018
@aabadie aabadie changed the title drivers/lps331ap: apply unified params definition scheme drivers/lps331ap: apply unified params definition scheme + cleanup Mar 8, 2018
@aabadie aabadie added the Process: API change Integration Process: PR contains or issue proposes an API change. Should be handled with care. label Mar 8, 2018
@aabadie
Copy link
Copy Markdown
Contributor Author

aabadie commented Apr 11, 2018

Just tested on iotlab-m3, works

@haukepetersen haukepetersen mentioned this pull request May 25, 2018
Closed
47 tasks
@aabadie aabadie force-pushed the pr/drivers/params/lps331ap branch from bcdf9d1 to c297236 Compare May 27, 2018 20:48
@aabadie aabadie changed the base branch from master to new_i2c_if May 27, 2018 20:48
@aabadie aabadie added TF: I2C Marks issues and PRs related to the work of the I²C rework task force and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels May 27, 2018
@aabadie aabadie force-pushed the pr/drivers/params/lps331ap branch from c297236 to f0141b7 Compare June 7, 2018 21:39
@aabadie aabadie force-pushed the pr/drivers/params/lps331ap branch from f0141b7 to ddfacf4 Compare June 18, 2018 11:32
dylad
dylad approved these changes Jul 4, 2018
View reviewed changes
Copy link
Copy Markdown
Member

@dylad dylad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Untested ACK.
It would be great if someone can test it.

@aabadie
Copy link
Copy Markdown
Contributor Author

aabadie commented Jul 4, 2018

It would be great if someone can test it.

Anyone with an iotlab-m3 or with an account on iot-lab can test this.

@dylad
Copy link
Copy Markdown
Member

dylad commented Jul 9, 2018

Let's merge it !

@dylad dylad merged commit 6c42f4c into RIOT-OS:new_i2c_if Jul 9, 2018
@jia200x
Copy link
Copy Markdown
Member

jia200x commented Jul 9, 2018 •
edited
Loading

just checked out this PR, try to compile the default example on iotlab-m3 and got this:

/home/jialamos/Development/RIOT/drivers/isl29020/isl29020.c:43:5: error: implicit declaration of function 'i2c_init_master'; did you mean 'i2c_write_bytes'? [-Werror=implicit-function-declaration]
     i2c_init_master(i2c, I2C_SPEED_NORMAL);
     ^~~~~~~~~~~~~~~
     i2c_write_bytes
/home/jialamos/Development/RIOT/drivers/isl29020/isl29020.c:47:11: error: too few arguments to function 'i2c_write_reg'
     res = i2c_write_reg(dev->i2c, address, ISL29020_REG_CMD, tmp);
           ^~~~~~~~~~~~~
In file included from /home/jialamos/Development/RIOT/drivers/include/isl29020.h:25:0,
                 from /home/jialamos/Development/RIOT/drivers/isl29020/isl29020.c:22:
/home/jialamos/Development/RIOT/drivers/include/periph/i2c.h:397:5: note: declared here
 int i2c_write_reg(i2c_t dev, uint16_t addr, uint16_t reg,
     ^~~~~~~~~~~~~
/home/jialamos/Development/RIOT/drivers/isl29020/isl29020.c: In function 'isl29020_read':
/home/jialamos/Development/RIOT/drivers/isl29020/isl29020.c:64:11: error: too few arguments to function 'i2c_read_reg'
     ret = i2c_read_reg(dev->i2c, dev->address, ISL29020_REG_LDATA, &low);
           ^~~~~~~~~~~~
In file included from /home/jialamos/Development/RIOT/drivers/include/isl29020.h:25:0,
                 from /home/jialamos/Development/RIOT/drivers/isl29020/isl29020.c:22:
/home/jialamos/Development/RIOT/drivers/include/periph/i2c.h:262:5: note: declared here
 int i2c_read_reg(i2c_t dev, uint16_t addr, uint16_t reg,
     ^~~~~~~~~~~~
/home/jialamos/Development/RIOT/drivers/isl29020/isl29020.c:65:12: error: too few arguments to function 'i2c_read_reg'
     ret += i2c_read_reg(dev->i2c, dev->address, ISL29020_REG_HDATA, &high);
            ^~~~~~~~~~~~
In file included from /home/jialamos/Development/RIOT/drivers/include/isl29020.h:25:0,
                 from /home/jialamos/Development/RIOT/drivers/isl29020/isl29020.c:22:
/home/jialamos/Development/RIOT/drivers/include/periph/i2c.h:262:5: note: declared here
 int i2c_read_reg(i2c_t dev, uint16_t addr, uint16_t reg,
     ^~~~~~~~~~~~
/home/jialamos/Development/RIOT/drivers/isl29020/isl29020.c: In function 'isl29020_enable':
/home/jialamos/Development/RIOT/drivers/isl29020/isl29020.c:82:11: error: too few arguments to function 'i2c_read_reg'
     res = i2c_read_reg(dev->i2c, dev->address, ISL29020_REG_CMD, &tmp);
           ^~~~~~~~~~~~
In file included from /home/jialamos/Development/RIOT/drivers/include/isl29020.h:25:0,
                 from /home/jialamos/Development/RIOT/drivers/isl29020/isl29020.c:22:
/home/jialamos/Development/RIOT/drivers/include/periph/i2c.h:262:5: note: declared here
 int i2c_read_reg(i2c_t dev, uint16_t addr, uint16_t reg,
     ^~~~~~~~~~~~
/home/jialamos/Development/RIOT/drivers/isl29020/isl29020.c:88:11: error: too few arguments to function 'i2c_write_reg'
     res = i2c_write_reg(dev->i2c, dev->address, ISL29020_REG_CMD, tmp);
           ^~~~~~~~~~~~~
In file included from /home/jialamos/Development/RIOT/drivers/include/isl29020.h:25:0,
                 from /home/jialamos/Development/RIOT/drivers/isl29020/isl29020.c:22:
/home/jialamos/Development/RIOT/drivers/include/periph/i2c.h:397:5: note: declared here
 int i2c_write_reg(i2c_t dev, uint16_t addr, uint16_t reg,
     ^~~~~~~~~~~~~
/home/jialamos/Development/RIOT/drivers/isl29020/isl29020.c: In function 'isl29020_disable':
/home/jialamos/Development/RIOT/drivers/isl29020/isl29020.c:103:11: error: too few arguments to function 'i2c_read_reg'
     res = i2c_read_reg(dev->i2c, dev->address, ISL29020_REG_CMD, &tmp);
           ^~~~~~~~~~~~
In file included from /home/jialamos/Development/RIOT/drivers/include/isl29020.h:25:0,
                 from /home/jialamos/Development/RIOT/drivers/isl29020/isl29020.c:22:
/home/jialamos/Development/RIOT/drivers/include/periph/i2c.h:262:5: note: declared here
 int i2c_read_reg(i2c_t dev, uint16_t addr, uint16_t reg,
     ^~~~~~~~~~~~
/home/jialamos/Development/RIOT/drivers/isl29020/isl29020.c:109:11: error: too few arguments to function 'i2c_write_reg'
     res = i2c_write_reg(dev->i2c, dev->address, ISL29020_REG_CMD, tmp);
           ^~~~~~~~~~~~~
In file included from /home/jialamos/Development/RIOT/drivers/include/isl29020.h:25:0,
                 from /home/jialamos/Development/RIOT/drivers/isl29020/isl29020.c:22:
/home/jialamos/Development/RIOT/drivers/include/periph/i2c.h:397:5: note: declared here
 int i2c_write_reg(i2c_t dev, uint16_t addr, uint16_t reg,
     ^~~~~~~~~~~~~
cc1: all warnings being treated as errors

Is it expected or due to the i2c work?

@dylad
Copy link
Copy Markdown
Member

dylad commented Jul 9, 2018

@jia200x This seems unrelated to this PR. But this is maybe related to the branch ?
All errors refers to isl29020.

@aabadie
Copy link
Copy Markdown
Contributor Author

aabadie commented Jul 9, 2018 •
edited
Loading

It's just that iotlab-m3 embeds a lot of adapted drivers (which might not be part of this PR). Best is not to try default but the specific driver application instead.

basilfx pushed a commit to basilfx/RIOT that referenced this pull request Jul 10, 2018
@dylad @basilfx
Merge pull request RIOT-OS#8757 from aabadie/pr/drivers/params/lps331ap
22613cb 
drivers/lps331ap: apply unified params definition scheme + cleanup
dylad added a commit to dylad/RIOT that referenced this pull request Jul 10, 2018
@dylad
Merge pull request RIOT-OS#8757 from aabadie/pr/drivers/params/lps331ap
926a354 
drivers/lps331ap: apply unified params definition scheme + cleanup
dylad added a commit that referenced this pull request Jul 11, 2018
@dylad
Merge pull request #8757 from aabadie/pr/drivers/params/lps331ap
f430ed4 
drivers/lps331ap: apply unified params definition scheme + cleanup
@aabadie aabadie deleted the pr/drivers/params/lps331ap branch September 23, 2018 08:47
@aabadie aabadie added this to the Release 2018.10 milestone Nov 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dylad dylad dylad approved these changes

Assignees

No one assigned

Labels

Area: drivers Area: Device drivers Process: API change Integration Process: PR contains or issue proposes an API change. Should be handled with care. TF: I2C Marks issues and PRs related to the work of the I²C rework task force Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation

Projects

None yet

Milestone

Release 2018.10

Development

Successfully merging this pull request may close these issues.

4 participants

@aabadie @dylad @jia200x @smlng