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.

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

Feature/add no addr reads #22

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

sfe-SparkFro
Copy link
Contributor

No description provided.

Copy link
Contributor Author

@sfe-SparkFro sfe-SparkFro left a comment

Choose a reason for hiding this comment

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

__i2c_rdwr__() needs changes

:return: response of read transaction
:rtype: list
"""
self._i2cbus.readBlock(address, write_message, read_nbytes)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This does not work in CircuitPython

# Custom method for reading +8-bit register using `i2c_msg` from `smbus2`
#
# Designed to have same operation as the __i2c_rdwr method in linux_i2c.py
def __i2c_rdwr__(self, address, write_message, read_nbytes):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This method should be changed. I know this is what was used elsewhere, but it's not a great name. Should match the conventions of the other methods in this driver, and get added to i2c_driver.py and linux_i2c.py. Then any device drivers that use it (I think just the VL53?) should be changed to use this new method. Does that make sense?

Copy link
Contributor

@santaimpersonator santaimpersonator Nov 20, 2024

Choose a reason for hiding this comment

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

The naming convention was based off the original i2c_rdwr() method from the smbus2 Python package that this package utilizes as the driver on Linux platforms:

def i2c_rdwr(self, *i2c_msgs):

When we added this method, I included the double underscores (__) to give it a private attribute. This keeps the method hidden (in a way), since we were only using it for the VL53L1x. Additionally, the method wasn't considered a feature of the package, as noted in the README.md file of the smbus2 GitHub repository, :

Note

Starting with v0.2, the smbus2 library also has support for combined read and write transactions. i2c_rdwr is not really a SMBus feature but comes in handy when the master needs to:

  1. read or write bulks of data larger than SMBus' 32 bytes limit.
  2. write some data and then read from the slave with a repeated start and no stop bit between.

Each operation is represented by a i2c_msg message object.


As a recommendation... I'd keep the __i2c_rdwr__()method for backwards compatibility. Since it is technically private, we could create a new public method with the naming convention that you prefer. To keep the functionality, you could:

  • Call on the __i2c_rdwr__()method
  • Reuse the same code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants