Skip to content

Conversation

MichelRottleuthner
Copy link
Contributor

Contribution description

Aapplies the new i2c API to the pn532.
Tested with adafruit PN532 RFID/NFC shield v1.3 and nucleo144-f446

Issues/PRs references

#6577

@dylad dylad added Area: drivers Area: Device drivers TF: I2C Marks issues and PRs related to the work of the I²C rework task force labels Jun 27, 2018
@dylad
Copy link
Member

dylad commented Jun 27, 2018

Overall changes are OK but I have no sensor for testing this.

@tcschmidt tcschmidt requested a review from MrKevinWeiss June 27, 2018 18:36
Copy link
Member

@smlng smlng left a comment

Choose a reason for hiding this comment

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

otherwise looks good, untested pre-ACK.

ret = i2c_write_bytes(dev->conf->i2c, PN532_I2C_ADDRESS, buff, len);
ret = i2c_write_bytes(dev->conf->i2c, PN532_I2C_ADDRESS, buff, len, 0);
if (ret == 0) {
ret = len;
Copy link
Member

Choose a reason for hiding this comment

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

looking at the SPI part this might also be ret = (int)len;, though my compiler didn't complain about assigned unsigned to signed - but might be different with other versions?!

ret = i2c_read_bytes(dev->conf->i2c, PN532_I2C_ADDRESS, buff, len + 1);
ret = i2c_read_bytes(dev->conf->i2c, PN532_I2C_ADDRESS, buff, len + 1, 0);
if (ret == 0) {
ret = len + 1;
Copy link
Member

Choose a reason for hiding this comment

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

dito, maybe cast to int needed, see above

@MrKevinWeiss
Copy link
Contributor

Tested on samr21-xpro and it seems like it is good. Ready to merge as soon as the casting and Travis issues are looked at!

@dylad dylad added the CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable label Jun 28, 2018
@dylad
Copy link
Member

dylad commented Jun 28, 2018

@MichelRottleuthner you can fix the remaining comments and squash directly so we can push the green button :)

@MichelRottleuthner MichelRottleuthner deleted the pr_pn532_new_i2c branch June 29, 2018 09:58
@dylad
Copy link
Member

dylad commented Jun 29, 2018

@MichelRottleuthner What's going on ?

@MichelRottleuthner
Copy link
Contributor Author

typo^^ just used git push origin :pr_pn532_new_i2c instead of git push origin pr_pn532_new_i2c

@MichelRottleuthner
Copy link
Contributor Author

is there no option to just reopen it o.O

@dylad
Copy link
Member

dylad commented Jun 29, 2018

Dunno :/ @smlng @miri64 I think we need your guidance with github here.

@dylad
Copy link
Member

dylad commented Jun 29, 2018

But I think you can reopen the PR and restore your branch using github.

@MichelRottleuthner
Copy link
Contributor Author

sadly no. Seems like the option was disabled because of me directly re-pushing it to the same name -.-

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: drivers Area: Device drivers CI: needs squashing Commits in this PR need to be squashed; If set, CI systems will mark this PR as unmergable TF: I2C Marks issues and PRs related to the work of the I²C rework task force
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants