search

google / periph

Older version of periph, see new version at https://github.com/periph
https://periph.io
Apache License 2.0
1.75k stars 167 forks source link

package mcp23xxx - Fixing according datasheet #452

Closed honzakadlec closed 3 years ago

honzakadlec commented 3 years ago

Notes

Please prefix the issue title with the primary package affected. For example, if this PR fixes periph.io/x/periph/host/sysfs, prefix the PR title with "sysfs:".

Please add new drivers under "experimental". Wonder what it takes to promote a driver as "stable"? See https://periph.io/project/#driver-lifetime-management A stable driver requires the smallest API surface, good unit test code coverage, good documentation and a page in https://github.com/periph/website/tree/master/site/content/device so it can show up at https://periph.io/device.

Mention the issue number it fixes or add the details of the changes if it doesn't have a specific issue.

Examples:

Fixes #12345 Helps with #12345 but doesn't not completely fix it.

googlebot commented 3 years ago

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

:memo: Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.

What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

google-cla[bot] commented 3 years ago

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

:memo: Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.

What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

honzakadlec commented 3 years ago

@googlebot I signed it!

google-cla[bot] commented 3 years ago

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google. In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

googlebot commented 3 years ago

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google. In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

codecov-io commented 3 years ago

Codecov Report

Merging #452 (7ca8b92) into master (d6dbb5a) will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           master    #452   +/-   ##
======================================
  Coverage    61.8%   61.8%           
======================================
  Files         126     126           
  Lines       11283   11283           
======================================
  Hits         6973    6973           
  Misses       4079    4079           
  Partials      231     231

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update d6dbb5a...7ca8b92. Read the comment docs.

maruel commented 3 years ago

Hi! Thanks for the change. It seems the commit is associated with the user "jan". Can you confirm? That's why the CLA check fail. I'd recommend a git commit --amend then git push -f.

Agreed on the stable migration, I plan to redo all that for next version but sadly have made no progress in the past months.

Do you have a link to a datasheet? There's none in the current package documentation.

maruel commented 3 years ago

Err ignore the comment about stable. :)

maruel commented 3 years ago

I looked at https://ww1.microchip.com/downloads/en/DeviceDoc/20001952C.pdf but failed to quickly spot the value, which page should I look at?

balazsgrill commented 3 years ago

I looked at https://ww1.microchip.com/downloads/en/DeviceDoc/20001952C.pdf but failed to quickly spot the value, which page should I look at?

@maruel page 12, Table 3-1, and page 17 table 3-5 (bank select is zero on reset). This is an error in my original contribution, thanks @honzakadlec for catching it!

maruel commented 3 years ago

Pushed 19c7e6b260b10719669122fa72fcbd1194e2f0b4 to replace this, since the CLA check was failing. Thanks!