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

refactor: deduplicate i2c parameters #188

Merged
merged 1 commit into from
Jun 5, 2024

Conversation

tdb3
Copy link
Contributor

@tdb3 tdb3 commented May 27, 2024

DS4432U, EMC2101, and INA2101 all contain the same macros for I2C bus usage.
This PR deduplicates code by pulling these i2c macros into a common header.

#define I2C_MASTER_SCL_IO 48        /*!< GPIO number used for I2C master clock */
#define I2C_MASTER_SDA_IO 47        /*!< GPIO number used for I2C master data  */
#define I2C_MASTER_NUM 0            /*!< I2C master i2c port number, the number of i2c peripheral interfaces available will depend on the chip */
#define I2C_MASTER_FREQ_HZ 400000   /*!< I2C master clock frequency */
#define I2C_MASTER_TX_BUF_DISABLE 0 /*!< I2C master doesn't need buffer */
#define I2C_MASTER_RX_BUF_DISABLE 0 /*!< I2C master doesn't need buffer */
#define I2C_MASTER_TIMEOUT_MS 1000

Also, since DS4432U.c was being touched, opportunistically added missing include for DS4432U.h.

Pulls common i2c macros (e.g. GPIO pins, speed, etc.)
into a common header, to deduplicate and increase
maintainability.
Also adds missing include for DS4432U.h in DS4432U.c.
Copy link
Collaborator

@Georges760 Georges760 left a comment

Choose a reason for hiding this comment

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

can you also please remove i2c_master_init(void) from DS4432.c and put it in the main file ?

@Georges760
Copy link
Collaborator

i planned to do such a PR myself, so I will help yours instead

@tdb3
Copy link
Contributor Author

tdb3 commented May 29, 2024

can you also please remove i2c_master_init(void) from DS4432.c and put it in the main file ?

Thank you very much for the review @Georges760!

Good idea. Does something like the following sound good to you?

  • move i2c_master_init() as well as i2c_driver_delete(), register_read(), and register_write_byte() to a common C file (e.g. i2c_common.c)
  • rename i2c_params.h to something like i2c_common.h

If so, I will make those changes (and any needed updates to CMakeLists

@Georges760
Copy link
Collaborator

@tdb3 are you able to do the additional commit ? I need it to move forward, sorry to push.

If you are busy, I can merge as it and do the additional change myself.

Tell me.

@tdb3
Copy link
Contributor Author

tdb3 commented Jun 4, 2024

@tdb3 are you able to do the additional commit ? I need it to move forward, sorry to push.

If you are busy, I can merge as it and do the additional change myself.

Tell me.

Thanks for pinging. Sorry, I'm a bit busy right now on another project. I saw a few more opportunities for deduplicating i2c functions beyond i2c_master_init() , i2c_driver_delete(), register_read(), and register_write_byte(). Don't let me hold you up. I could convert this PR to draft if you'd like or you could merge as-is and one of us could open a follow up PR later.

@Georges760 Georges760 merged commit abe6d28 into skot:master Jun 5, 2024
1 check passed
aaron3481 pushed a commit to aaron3481/ESP-Miner that referenced this pull request Jun 11, 2024
refactor: deduplicate i2c parameters
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.

2 participants