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

power: ades1754 : Add support for ADES1754 #2429

Merged
merged 4 commits into from
Feb 7, 2025
Merged

power: ades1754 : Add support for ADES1754 #2429

merged 4 commits into from
Feb 7, 2025

Conversation

RaduSabau1
Copy link
Collaborator

Pull Request Description

The ADES1754/ADES1755/ADES1756 are flexible
data-acquisition systems for the management of high-
voltage and low-voltage battery modules. The systems
can measure 14 cell voltages and a combination of six
temperatures or system voltage measurements with fully
redundant measurement engines in 162μs, or perform
all inputs solely with the ADC measurement engine in
99μs. Fourteen internal balancing switches rated
for >300mA for cell-balancing current support extensive
built-in diagnostics. Up to 32 devices can be daisy-
chained to manage 448 cells and monitor 192
temperatures.

This PR includes examples that use other device drivers, therefore it needs to be merged after #2428

PR Type

  • Bug fix (change that fixes an issue)
  • New feature (change that adds new functionality)
  • Breaking change (has dependencies in other repos or will cause CI to fail)

PR Checklist

  • I have followed the Coding style guidelines
  • I have performed a self-review of the changes
  • I have commented my code, at least hard-to-understand parts
  • I have build all projects affected by the changes in this PR
  • I have tested in hardware affected projects, at the relevant boards
  • I have signed off all commits from this PR
  • I have updated the documentation (wiki pages, ReadMe etc), if applies

dbogdan
dbogdan previously approved these changes Feb 3, 2025
@dbogdan
Copy link
Contributor

dbogdan commented Feb 3, 2025

cppcheck complain is valid, @RaduSabau1, please have a look.

@RaduSabau1 RaduSabau1 force-pushed the staging/ades175x branch 2 times, most recently from 81e9dbe to a3b737b Compare February 3, 2025 09:19
@RaduSabau1
Copy link
Collaborator Author

V2:

  • Fixed cppcheck failure.
  • Fixed documentation errors.

@RaduSabau1
Copy link
Collaborator Author

V3:

  • Add support for ADES1751 and ADES1752.

@RaduSabau1
Copy link
Collaborator Author

V3:

  • Update project name to correspond with latest project structure.

@RaduSabau1
Copy link
Collaborator Author

V4:

  • Fix CI documentation failure.

projects/ades1754/builds.json Outdated Show resolved Hide resolved
@RaduSabau1 RaduSabau1 force-pushed the staging/ades175x branch 2 times, most recently from 00a30da to 8167de5 Compare February 4, 2025 08:48

struct ades1754_init_param {
struct no_os_uart_init_param *uart_param;

Copy link
Contributor

Choose a reason for hiding this comment

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

remove all these blank lines


struct ades1754_desc {
struct no_os_uart_desc *uart_desc;

Copy link
Contributor

Choose a reason for hiding this comment

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

remove blank lines

case 0:
tx_data[i + 9] = ades1754_manchester(no_os_field_get(ADES1754_LOWER_NIBBLE_MASK,
ADES1754_FILL_BYTE_C2), true);

Copy link
Contributor

Choose a reason for hiding this comment

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

remove blank lines for all cases

for (i = 3; i < 3 + 2 * desc->no_dev; i += 4) {
data[j] = no_os_field_prep(ADES1754_MSB_MASK,
no_os_bit_swap_constant_8(crc[i + 1])) | no_os_bit_swap_constant_8(crc[i]);

Copy link
Contributor

Choose a reason for hiding this comment

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

this one also seems unnecessary

mask = NO_OS_GENMASK(15, 2);
reg = ADES1754_OVTHCLR_REG;
reg2 = ADES1754_OVTHSET_REG;

Copy link
Contributor

Choose a reason for hiding this comment

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

remove blank lines


ret = ades1754_init(&ades1754_desc, &ades1754_ip);
if (ret)
return ret;
Copy link
Contributor

Choose a reason for hiding this comment

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

perhaps provide one or two more API calls after the init to set something, whatever you want

The initialization data used in the examples is taken out from:
`Project Common Data Path <https://github.com/analogdevicesinc/no-OS/tree/main/projects/ades1754/src/common>`_

The macros used for Common Data structures are defined in pkatform specific files found at:
Copy link
Contributor

Choose a reason for hiding this comment

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

pkatform

@@ -0,0 +1,104 @@
/***************************************************************************//**
* @file def_uart_example.c
Copy link
Contributor

Choose a reason for hiding this comment

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

i don't really like the default name, you probably need better names for the examples


EXAMPLE ?= def_uart_example

SPI UART Example
Copy link
Contributor

Choose a reason for hiding this comment

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

really, this example name should be more telling, SPI UART doesnt make sense to me, but SPI UART bridge does

The macros used for Common Data structures are defined in pkatform specific files found at:
`Project Platform Configuration Path <https://github.com/analogdevicesinc/no-OS/tree/main/projects/ades1754/src/platform>`_

DEF UART Example
Copy link
Contributor

Choose a reason for hiding this comment

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

And this example name tells me nothing about what this example is about

Add initial header and source files for ADES1754 driver.

Signed-off-by: Radu Sabau <[email protected]>
Add initial README.rst documentation file for ADES1754 driver.

Signed-off-by: Radu Sabau <[email protected]>
Add initial header and source files for ADES1754 project, alongside
Makefiles.

Signed-off-by: Radu Sabau <[email protected]>
Add README.rst documentation file for ADES1754 project.

Signed-off-by: Radu Sabau <[email protected]>
@RaduSabau1
Copy link
Collaborator Author

V5:

  • Removed extra blank lines.
  • Renamed examples.
  • Addressed other comments.

@dbogdan dbogdan merged commit 7b45001 into main Feb 7, 2025
11 of 14 checks passed
@dbogdan dbogdan deleted the staging/ades175x branch February 7, 2025 12:55
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