Skip to content

Driver for Freescale MPL3115A2 Pressure/Altitude Sensor#2123

Merged
PeterKietzmann merged 2 commits intoRIOT-OS:masterfrom
jfischer-no:pr@mpl3115a2
Jan 7, 2015
Merged

Driver for Freescale MPL3115A2 Pressure/Altitude Sensor#2123
PeterKietzmann merged 2 commits intoRIOT-OS:masterfrom
jfischer-no:pr@mpl3115a2

Conversation

@jfischer-no
Copy link
Copy Markdown
Contributor

@jfischer-no jfischer-no commented Dec 3, 2014

This PR add support for Freescale MPL3115A2 Pressure/Altitude Sensor.
This driver only implements pressure and temperature measurement functionality.

@OlegHahm OlegHahm added Area: drivers Area: Device drivers Type: new feature The issue requests / The PR implemements a new feature for RIOT labels Dec 3, 2014
PeterKietzmann
PeterKietzmann reviewed Dec 3, 2014
View reviewed changes
drivers/include/mpl3115a2.h Outdated
Copy link
Copy Markdown
Member

@PeterKietzmann PeterKietzmann Dec 3, 2014

Choose a reason for hiding this comment

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

What about the documentation for the return value?

@PeterKietzmann
Copy link
Copy Markdown
Member

PeterKietzmann commented Dec 3, 2014

This one looks also good to me. If you provide us with the hardware I will test and (probably) ACK. This applies to #2119 and #2121 also.

@PeterKietzmann
Copy link
Copy Markdown
Member

PeterKietzmann commented Dec 9, 2014

The temperature is ~250 °F °C I think that is not correct. The pressure seems reasonable. The application runs fine.

@PeterKietzmann
Copy link
Copy Markdown
Member

PeterKietzmann commented Dec 9, 2014

Same as for your other drivers, waiting for vtimer bugfix.

@PeterKietzmann
Copy link
Copy Markdown
Member

PeterKietzmann commented Dec 9, 2014

Needs squashing. Take care of the word "fix" in the final commit message. Also here we should maybe wait until #2059 is merged.

@PeterKietzmann
Copy link
Copy Markdown
Member

PeterKietzmann commented Dec 9, 2014

And please eliminate doxygen warnings:

mpl3115a2.h:139: warning: argument 'press' of command @param is not found in the argument list of mpl3115a2_read_pressure(mpl3115a2_t *dev, uint32_t *pres, uint8_t *status)
mpl3115a2.h:139: warning: The following parameters of mpl3115a2_read_pressure(mpl3115a2_t *dev, uint32_t *pres, uint8_t *status) are not documented:
  parameter 'pres'

@PeterKietzmann PeterKietzmann mentioned this pull request Dec 9, 2014
Merged
@jonas-rem
Copy link
Copy Markdown
Contributor

jonas-rem commented Dec 9, 2014

Thanks for testing.
To get the actual termperature value one has to divide the return value by 10, so it should be 25°C. Thereby we could avoid the use of float. A Temp value with a precision of 0.1°C should be sufficient.

@OlegHahm
Copy link
Copy Markdown
Member

OlegHahm commented Dec 9, 2014

Getting rid of floats wherever possible sounds good for me.

@OlegHahm
Copy link
Copy Markdown
Member

OlegHahm commented Dec 9, 2014

I only get

2014-12-09 16:20:47,098 - INFO # Initializing MPL3115A2 sensor at I2C_0... [OK]
2014-12-09 16:20:47,098 - INFO # 
2014-12-09 16:20:47,101 - INFO # Pressure: 0 Status: 00
2014-12-09 16:20:47,103 - INFO # Temperature: 0

@PeterKietzmann
Copy link
Copy Markdown
Member

PeterKietzmann commented Dec 9, 2014

Did you comment out vtimer_usleep(SLEEP); in the main.c ? vtimer does not work properly up to now

@OlegHahm
Copy link
Copy Markdown
Member

OlegHahm commented Dec 9, 2014

So, the first x measurements are always failing? But, yes, after removing this I get sensible results.

@miri64
Copy link
Copy Markdown
Member

miri64 commented Dec 10, 2014

OT: Can we please rename the PR to something more descriptive? I get confused everytime I get notified about this PR.

@jfischer-no jfischer-no changed the title pr@mpl3115a2 Driver for Freescale MPL3115A2 Pressure/Altitude Sensor Dec 10, 2014
@jfischer-no
Copy link
Copy Markdown
Contributor Author

jfischer-no commented Dec 11, 2014

@PeterKietzmann At the moment I do not see any possibility to get vtimer working. I hope using of hwtimer instead of vtimer in test application is ok. Can you test it please. (you need 9cec32b).

PeterKietzmann
PeterKietzmann reviewed Dec 12, 2014
View reviewed changes
tests/driver_mpl3115a2/main.c Outdated
Copy link
Copy Markdown
Member

@PeterKietzmann PeterKietzmann Dec 12, 2014

Choose a reason for hiding this comment

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

remove tab and align "correctly"

@PeterKietzmann
Copy link
Copy Markdown
Member

PeterKietzmann commented Dec 12, 2014

This works. For me it is okay to use hwtimer instead. When ther is a vtimer in the future we can also change it. However, lets wait with this PR until #2059 is ready. OK?

@jfischer-no
Copy link
Copy Markdown
Contributor Author

jfischer-no commented Dec 12, 2014

Can/Should I replace vtimer with hwtimer_wait in #2119 #2121 #2148 too?
We are working on #2059, it could be ready next week...

@PeterKietzmann
Copy link
Copy Markdown
Member

PeterKietzmann commented Dec 12, 2014

Yes, do so. If someone is against that: please tell us now! Otherwise we'll do a PR that changes to vtimer when is exists.

@OlegHahm OlegHahm added the Community: Hack'n'ACK candidate This PR is a candidate for review and discussion during one of RIOT's monthly Hack'n'ACK parties label Jan 6, 2015
@PeterKietzmann
Copy link
Copy Markdown
Member

PeterKietzmann commented Jan 7, 2015

vtimer works now. Needs squashing and waiting for #2059 to be merged. Then I'll give my final ACK.

@OlegHahm
Copy link
Copy Markdown
Member

OlegHahm commented Jan 7, 2015

Ready to go.

@PeterKietzmann PeterKietzmann mentioned this pull request Jan 7, 2015
Merged
@PeterKietzmann
Copy link
Copy Markdown
Member

PeterKietzmann commented Jan 7, 2015

okay, go

PeterKietzmann added a commit that referenced this pull request Jan 7, 2015
@PeterKietzmann
Merge pull request #2123 from jfischer-phytec-iot/pr@mpl3115a2
2fb2f49 
Driver for  Freescale MPL3115A2 Pressure/Altitude Sensor
@PeterKietzmann PeterKietzmann merged commit 2fb2f49 into RIOT-OS:master Jan 7, 2015
@jfischer-no jfischer-no deleted the pr@mpl3115a2 branch January 31, 2015 10:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@PeterKietzmann PeterKietzmann

Labels

Area: drivers Area: Device drivers Community: Hack'n'ACK candidate This PR is a candidate for review and discussion during one of RIOT's monthly Hack'n'ACK parties Type: new feature The issue requests / The PR implemements a new feature for RIOT

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@jfischer-no @PeterKietzmann @jonas-rem @OlegHahm @miri64