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

pi4io3 Driver Agnostic #187

Closed
wants to merge 1 commit into from
Closed

pi4io3 Driver Agnostic #187

wants to merge 1 commit into from

Conversation

8q8b
Copy link

@8q8b 8q8b commented Oct 27, 2024

Changes

Replaced HAL specific code with generalized code with pointers for HAL functionality.

Closes #159

@8q8b 8q8b requested a review from dyldonahue October 27, 2024 21:09
@8q8b 8q8b self-assigned this Oct 27, 2024
Copy link
Contributor

@dyldonahue dyldonahue left a comment

Choose a reason for hiding this comment

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

looks pretty much good, a few small tweaks

HAL_StatusTypeDef pi4ioe_init(pi4ioe_t *gpio, I2C_HandleTypeDef *i2c_handle)
typedef (*I2C_WriteFuncPtr)(uint16_t device_addr, uint8_t *data, uint16_t size);
typedef (*I2C_ReadFuncPtr)(uint16_t device_addr, uint8_t *reg,
uint16_t reg_size, uint8_t *data,
Copy link
Contributor

Choose a reason for hiding this comment

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

not sure that reg size is needed here. Both read na dwrite cna just take "data, reg, length)

@@ -23,49 +23,61 @@
uint8_t oreg_1_data;
uint8_t oreg_2_data;

HAL_StatusTypeDef pi4ioe_init(pi4ioe_t *gpio, I2C_HandleTypeDef *i2c_handle)
typedef (*I2C_WriteFuncPtr)(uint16_t device_addr, uint8_t *data, uint16_t size);
typedef (*I2C_ReadFuncPtr)(uint16_t device_addr, uint8_t *reg,
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpick but can the names be changed :

I2C_WriteFuncPtr --> WritePtr
... --> ReadPtr

i2c_write --> write
i2c_read --> read

uint16_t data_size);

typedef struct {
uint16_t i2c_addr; // I2C address of the GPIO expander
Copy link
Contributor

Choose a reason for hiding this comment

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

this can be handled at app layer, we dont need tos tore the i2c address here

@dyldonahue
Copy link
Contributor

Look at Caio's PR in MSB FW as a good example for implementing the new style driver

@dyldonahue dyldonahue closed this Dec 8, 2024
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.

[Driver] - Make Pi4io3 driver platform agnostic
2 participants