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

added read write and init #68

Merged
merged 2 commits into from
Feb 19, 2024
Merged

added read write and init #68

merged 2 commits into from
Feb 19, 2024

Conversation

dyldonahue
Copy link
Contributor

Changes

Built a drive to handle coms to the gpio exander chip

Notes

Just by way of needing function pointers have common parameters, this structure forces the application to have a wrapper for its i2c reads/writes. For example, using HAL, wed have to have the read/write functions we pass in internally handle i2c_handle. Not sure theres a way around this, just something to note though

Test Cases

Not tested/verified

To Do

IDK if we want to abstract any more APIs, for shepherd i dont see the need

Checklist

It can be helpful to check the Checks and Files changed tabs.
Please reach out to your Project Lead if anything is unclear.
Please request reviewers and ping on slack only after you've gone through this whole checklist.

  • [x ] No merge conflicts
  • [x ] All checks passing
  • [ x] Remove any non-applicable sections of this template
  • [ x] Assign the PR to yourself
  • [ x] Request reviewers & ping on Slack
  • [ x] PR is linked to the ticket (fill in the closes line below)

Closes # (#65)

@dyldonahue dyldonahue requested a review from nwdepatie January 30, 2024 03:27
@dyldonahue dyldonahue self-assigned this Jan 30, 2024
Copy link
Contributor

@nwdepatie nwdepatie left a comment

Choose a reason for hiding this comment

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

Feels a bit yucky, look at my comments. I think you're def picking up on the general vibe I said tho for function pointers

#include <stdint.h>

typedef struct {
int (*write)(uint8_t addr, const uint8_t *data, uint8_t len);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we make these typedefs to make this less verbose? Or maybe like a macro? This looks yucky and I feel like there's a better solution.

Maybe a good idea is to create typedefs for eeprom_read or eeprom_write and then the user can pass them in? Or maybe if you wanna make it more generic make a typedef for I2C_Read/write or Reg_read/write?

@dyldonahue dyldonahue requested a review from nwdepatie February 11, 2024 20:43
Copy link
Contributor

@nwdepatie nwdepatie left a comment

Choose a reason for hiding this comment

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

Tested, and approved

@nwdepatie nwdepatie merged commit 3e9ba18 into main Feb 19, 2024
1 check passed
@dyldonahue dyldonahue deleted the gpio-expander branch March 25, 2024 01:59
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