Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Driver for Freescale MAG3110 Magnetometer #2121

Merged
merged 2 commits into from
Jan 9, 2015
Merged

Driver for Freescale MAG3110 Magnetometer #2121

merged 2 commits into from
Jan 9, 2015

Conversation

jfischer-no
Copy link
Contributor

This PR add support for Freescale MAG3110 3-Axis Digital Magnetometer.

@LudwigKnuepfer LudwigKnuepfer added Type: new feature The issue requests / The PR implemements a new feature for RIOT Area: drivers Area: Device drivers labels Dec 2, 2014
@PeterKietzmann
Copy link
Member

Looks fine. Has someone the hardware by chance?

PeterKietzmann
PeterKietzmann reviewed Dec 3, 2014
View reviewed changes
drivers/include/mag3110.h
*
* @param[in] dev device descriptor of magnetometer
* @param[out] dtemp die temperature
*/
Copy link
Member

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 PeterKietzmann mentioned this pull request Dec 3, 2014
Merged
@PeterKietzmann
Copy link
Member

Is it normal that a magnet above the sensor gives a negative magnitude on the z-axis?

@PeterKietzmann
Copy link
Member

This one works fine except for the vtimer. When the timer is fixed and the commits are squashed I give an ACK. BTW: The commit message should not include the word "fix" (compare Travis result). Maybe we should wait with this until #2059 is merged.

@LudwigKnuepfer
Copy link
Member

Although there is no official guideline: please use some module: change scheme for commit messages.
E.g.: drivers/mag3110: initial import.

@LudwigKnuepfer LudwigKnuepfer added the CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable label Dec 9, 2014
@LudwigKnuepfer
Copy link
Member

The fix... commit message is fine - but it should be merged with the commit that contains the deficit prior to merging the PR.

This was referenced Dec 9, 2014
Merged
Merged
@jfischer-no jfischer-no changed the title pr@mag3110 Driver for Freescale MAG3110 Magnetometer Dec 10, 2014
@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
Member

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

@OlegHahm
Copy link
Member

OlegHahm commented Jan 7, 2015

I think we are ready to go.

@PeterKietzmann
Copy link
Member

@LudwigOrtmann I think I didn't get what you meant above. Are you willing to merge after the conflicts are solved?

@PeterKietzmann
Copy link
Member

PS: Without looking deeper in it I assume you just need to rebase and solve conflicts in drivers/Makefile.include

@LudwigKnuepfer LudwigKnuepfer removed the CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable label Jan 8, 2015
@LudwigKnuepfer
Copy link
Member

@PeterKietzmann
Judging from the chronological order there was some issue with the commit messages which has been resolved since my comment.

Why should this PR wait for #2059?
As far as I can tell the driver is independent from the board.

@LudwigKnuepfer
Copy link
Member

@jfischer-phytec-iot please rebase / resolve the conflicts.

@PeterKietzmann
Copy link
Member

I was just thinking it's illogical to merge this even if the (only) board which has this sensor is not merged to master. But generally it's independent, that's why we already merged two other sensor drivers of the phyWave board. After rebase I'll finally ACK :-)

@jfischer-no
Copy link
Contributor Author

I would rebase thies PR if #2119 will be merged.

@PeterKietzmann
Copy link
Member

#2119 is merged

@jfischer-no
Copy link
Contributor Author

@PeterKietzmann rebased

@PeterKietzmann
Copy link
Member

Jippie. And go

PeterKietzmann added a commit that referenced this pull request Jan 9, 2015
@PeterKietzmann
Merge pull request #2121 from jfischer-phytec-iot/pr@mag3110
52dbaa5 
Driver for Freescale MAG3110 Magnetometer
@PeterKietzmann PeterKietzmann merged commit 52dbaa5 into RIOT-OS:master Jan 9, 2015
@jfischer-no jfischer-no deleted the pr@mag3110 branch January 31, 2015 10:32
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.
4 participants
@jfischer-no @PeterKietzmann @LudwigKnuepfer @OlegHahm