adafruit / Adafruit_CircuitPython_INA260

CircuitPython driver for the TI INA260 current and power sensor
MIT License
14 stars 11 forks source link

Adding fields #15

Closed gpongelli closed 3 years ago

gpongelli commented 3 years ago

Explicitly adding all the MASK_ENABLE bits, MANUFACTURER and REVISION reg.

siddacious commented 3 years ago

Thank you for your contribution! This particular library is near and dear to me, so it's great to see someone else chipping in!

gpongelli commented 3 years ago

Thank you for this library!

I'm working on this chip with a custom board (not the Adafruit one) and this library is helping me a lot, instead of writing similar code to yours 😃

siddacious commented 3 years ago

It looks like you're to the Black formatting check, almost there! See here for more info: https://learn.adafruit.com/improve-your-code-with-pylint/black

gpongelli commented 3 years ago

@siddacious sorry but I do not understand what's still missing, could you help me ?

siddacious commented 3 years ago

@gpongelli Please try to reproduce the sphinx build on your machine to see if you get the same errors.

You can use his guide for reference: https://learn.adafruit.com/creating-and-sharing-a-circuitpython-library/testing-with-github-actions

gpongelli commented 3 years ago

@siddacious was the pipeline executed before my branch did start ? because I'm doing fixes over fixes on file part I've not touched, like the following rows from master commit

**Hardware:**

* `INA260 Breakout <https://www.adafruit.com/products/4226>`_

**Software and Dependencies:**

* Adafruit CircuitPython firmware for the supported boards:
  https://github.com/adafruit/circuitpython/releases

 * Adafruit's Bus Device library: https://github.com/adafruit/Adafruit_CircuitPython_BusDevice

 * Adafruit's Register library: https://github.com/adafruit/Adafruit_CircuitPython_Register
siddacious commented 3 years ago

@gpongelli You're likely running into checks that were added after the last time a build was done on this repo.

Your help updating the code would be much appreciated

gpongelli commented 3 years ago

@siddacious checks pass now

siddacious commented 3 years ago

@gpongelli please see my earlier request for change where I ask for an example that tests the new code. It will help me test as well as help users understand how to use the new features

gpongelli commented 3 years ago

@siddacious look at the new example file and let me know

gpongelli commented 3 years ago

@siddacious @ladyada anyone there?

ladyada commented 3 years ago

hihi please adjust the PR so CI passes https://github.com/adafruit/Adafruit_CircuitPython_INA260/runs/1862521113 we have a guide on linting https://learn.adafruit.com/improve-your-code-with-pylint @dherrada can help if you're not sure what to do

gpongelli commented 3 years ago

Yes thanks, ‘cause it’s not clear how a commit with only empty rows to fix a merge conflict can not pass the pipeline.

Has something changed into pipeline’s checks during last month?

Il giorno 15 feb 2021, alle ore 18:38, ladyada notifications@github.com ha scritto:

 hihi please adjust the PR so CI passes https://github.com/adafruit/Adafruit_CircuitPython_INA260/runs/1862521113 we have a guide on linting https://learn.adafruit.com/improve-your-code-with-pylint @dherrada can help if you're not sure what to do

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

ladyada commented 3 years ago

yes we now require copyright deets!

gpongelli commented 3 years ago

@ladyada copyrights added after rebase, but still fails due to something runner-related issue.

ladyada commented 3 years ago

weird - dunno what that error is. @tannewt @dherrada plz take a look when yr around next

evaherrada commented 3 years ago

@ladyada @gpongelli Looks like an issue with the latest Pylint (https://github.com/PyCQA/pylint/issues/4097) 2.6.1. Should be fixed by just running the CI again when 2.6.2 gets released

Edit: looks like 2.6.2 has been released.

evaherrada commented 3 years ago

Yeah, looks like it was just a bug in 2.6.1. Seems to be fixed now.

gpongelli commented 3 years ago

Pipeline now passes.

There’s still an old code review request already addressed.

ladyada commented 3 years ago

aawesome

gpongelli commented 3 years ago

thanks for merging this PR.

is it planned a 1.2.7 version sooner or later ?

ladyada commented 3 years ago

there are version release sweeps once a week

evaherrada commented 3 years ago

@gpongelli Yep. This should be released tomorrow.