adafruit / Adafruit-Raspberry-Pi-Python-Code

Adafruit library code for Raspberry Pi
1.43k stars 686 forks source link

Added TSL2561 directory #139

Closed IainColledge closed 4 years ago

IainColledge commented 8 years ago

Pulled together a working and up to date python driver for the TSL2561 Lux sensor.

Has been compared to a 10 UKP Lux meter and was within 10% across the range.

csatt commented 8 years ago

Hi Iain,

Thanks for doing this - it's just what I was looking for!

I have played with your code, and there are a couple bugs and some other more cosmetic issues. I posted the same list on the Adafruit forum thread.

Bug #1: The class name has the middle two digits transposed

    Adafruit_TSL2651 should be Adafruit_TSL2561

Bug #2: The read8 and read16 methods (functions) call the I2C readS8 and readS16 methods respectively. They should call the readU8 and readU16 (i.e. unsigned) methods.

    ==> This is why you needed the hack that you added. The readS16
        method was returning a negative number when the register had
        its upper bit set. You can remove the hack when this bug is
        fixed.

Other minor issues:

IainColledge commented 8 years ago

Thanks very much, will update and do another pull request, really appreciated.

IainColledge commented 8 years ago

Tidied up the code so it looks like Python now, just adding some things like throwing an exception for saturation and adding handlers to calling code.

IainColledge commented 8 years ago

This is about it for now as don't have the time at moment to go into the deeper issues.

csatt commented 8 years ago

Hi Iain,

I forked from the latest version of your fork, and made several changes. I am new to GitHub, but I believe that I can now submit a pull request and it will go to you and not to the Adafruit repo owner, since my fork is off of your fork. We'll see!

You should be able to see the diffs in the pull request. A few of these are things that I mentioned in my feedback to you on your original version. Let me know if there are any you disagree with and I can make changes and re-submit the pull request. Here's the summary:

- Changed Adafruit_I2C.py from a copy of
  ../Adafruit_I2C/Adafruit_I2C.py to a symbolic link

  I'm sure your pull request would be rejected without this change.

- Added underscore back into class name

  You were probably trying to conform to a coding standard you read
  about. But I think it is more important to conform to the naming
  conventions that are already used in the Adafruit library.
  Unfortunately there are two such conventions. The class name
  should either be Adafruit_TSL2561 or just TSL2561. I prefer the
  former since it matches the name of the file.

- Removed unnecessary inheritance from Adafruit_I2C

  You had a comment wondering if this inheritance was
  necessary. It's not. I removed the comment and changed it to the
  generic "object" inheritance.

- Removed vestigial trailing */ from comments

  I'd mentioned this in my earlier comments.

- Removed (now unnecessary) autogain hack

  The hack was only necessary when you had the signed/unsigned bug.
  Now it works fine without the hack, so I removed it.

- Fold (most) long lines to comply with col 80 limit

  This is just a cosmetic change, but better made now than
  later. The 80-column convention has archaic roots (punch cards!),
  but still is nice so the code is readable in a narrow window
  (expecially when doing a side-by-side diff).

- Enhanced the saturation exception so the message text says which
  channel (or both) was saturated

- Added BSD license header comment

  In theory, all code in the Adafruit repository should have this
  boilerplate header. Having it increases your chances of getting
  the pull request accepted.

You also had the following comment before the exception code:

"TODO: Fix that exception not raised when hits saturation but returns a Lux of around 780"

I removed that comment because I did not observe that problem. I tested it in all of the different modes and was able to get the saturation exception in all cases (broadband only).

P.S. I am going to be gone for the next 5 days. I may see e-mail but won't be able to make (or more importantly, test) any code changes.

IainColledge commented 8 years ago

Chris, thanks very much, I've merged your pull request and afaik given you permissions to merge anything into this pull request in the future, you have write permission to my fork and this pull.

I'll try the saturation test again which is basically holding a maglite above the sensor until it max's out to see if the hack is needed for me or not. Will let you know what happens.

The coding standards were basically from PEP 8 and suing Pycharm & SonarQube but meh am not fussed, it at least looks Pythonish vs C++ now ;)

Have a great 5 days away and if I don't hear from you a great x-mas, I'll be available until about the 5th. Would be good to put this sucker to bed.

csatt commented 8 years ago

Hi Iain,

Looks like the fork-of-a-fork thing worked. Good to know! Thanks for giving me push permission. That's something else I haven't done before, but I'm sure it's straightforward if I need to use it.

It will be interesting to hear what you find when you try the saturation test again. Your test sounds pretty similar to mine. It's easiest to hit the saturation point in the 402ms integration time mode because it happens at a lower lux value, but I was able to hit it in all three modes if I got the light close enough. The exception seemed to work fine.

I'm not likely to contribute much more to this since my project is working now, and it was just a personal project anyway. I think the code is in good shape and I hope your pull request is granted.

I hope you have a great Christmas as well!

IainColledge commented 8 years ago

Will let you know and thanks, will update my code for the dataloop.io interface to use the new class name.

Iain

ladyada commented 4 years ago

Thank you for the Pull Request This library has been deprecated in favor of our python3 Blinka library. We have replaced all of the libraries that use this repo with CircuitPython libraries that are Python3 compatible, and support a wide variety of single board/linux computers!

Visit https://circuitpython.org/blinka for more information

CircuitPython has support for almost 200 different drivers, and a as well as FT232H support for Mac/Win/Linux!