Dev/ade7978 #2400
Open
Dev/ade7978 #2400
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
raezt
force-pushed
the
dev/ade7978
branch
8 times, most recently
from
08f443c to
dcc7207
Compare
raezt
requested review from
buha,
CiprianRegus,
dbogdan,
amiclaus,
rbolboac and
danmois
as code owners
drivers/meter/ade7978/ade7978.c
Outdated
Comment on lines
72
to
73
| if ((reg_addr >= ADE7978_REG_MMODE && reg_addr <= ADE7978_REG_LAST_RWDATA8) || | ||
| (reg_addr >= ADE7978_REG_CONFIG2 && reg_addr <= ADE7978_SET_SPI_ADDR)) { |
There was a problem hiding this comment.
I would define a macro for these conditions, since they do repeat.
|
|
||
| /* Hard reset the device */ | ||
| if (!init_param.reset_desc) | ||
| goto error_dev; |
There was a problem hiding this comment.
ret is not initialized at this moment.
drivers/meter/ade7978/ade7978.c
Outdated
Comment on lines
185
to
170
| if (!dev) | ||
| return -ENODEV; | ||
| if (!status) | ||
| return -EINVAL; |
There was a problem hiding this comment.
You can remove these, since they are checked in the ade7978_read() call below.
Comment on lines
87
to
88
| temperature = (float) 8.72101 * (float)(0.00001) * | ||
| (float) dev->temperature - (float) 306.47; |
There was a problem hiding this comment.
No need to cast anything here.
drivers/meter/ade7978/ade7978.h
Outdated
Comment on lines
47
to
49
| /******************************************************************************/ | ||
| /********************** Macros and Constants Definitions **********************/ | ||
| /******************************************************************************/ |
There was a problem hiding this comment.
We try to not have these comments in new drivers, since they don't really add any value.
Comment on lines
129
to
134
| if (val == NO_OS_GPIO_LOW) | ||
| val = NO_OS_GPIO_HIGH; | ||
| else | ||
| val = NO_OS_GPIO_LOW; | ||
|
|
||
| return no_os_gpio_set_value(gpio_led_desc, val); |
There was a problem hiding this comment.
Can be replaced with
return no_os_gpio_set_value(gpio_led_desc, !val);
drivers/meter/ade7978/ade7978.c
Outdated
| return ret; | ||
|
|
||
| // /* Enable the DSP */ | ||
| // return ade7978_write(dev, ADE7978_REG_RUN, ADE7978_RUN_ON); |
There was a problem hiding this comment.
You can remove this line.
drivers/meter/ade7978/ade7978.c
Outdated
| return ret; | ||
| } | ||
|
|
||
| // Dumb read of energy registers to erase their contents |
There was a problem hiding this comment.
The comment style should be consistent throughout No-OS. Use /**/ instead.
projects/eval-ade7978/src.mk
Outdated
Comment on lines
4
to
23
| INCS += $(INCLUDE)/no_os_alloc.h \ | ||
| $(INCLUDE)/no_os_delay.h \ | ||
| $(INCLUDE)/no_os_error.h \ | ||
| $(INCLUDE)/no_os_gpio.h \ | ||
| $(INCLUDE)/no_os_print_log.h \ | ||
| $(INCLUDE)/no_os_spi.h \ | ||
| $(INCLUDE)/no_os_timer.h \ | ||
| $(INCLUDE)/no_os_mutex.h \ | ||
| $(INCLUDE)/no_os_util.h \ | ||
| $(INCLUDE)/no_os_lf256fifo.h \ | ||
| $(INCLUDE)/no_os_list.h \ | ||
| $(INCLUDE)/no_os_irq.h \ | ||
| $(INCLUDE)/no_os_units.h \ | ||
| $(INCLUDE)/no_os_init.h \ | ||
| $(INCLUDE)/no_os_alloc.h \ | ||
| $(INCLUDE)/no_os_crc8.h \ | ||
| $(INCLUDE)/no_os_crc16.h \ | ||
| $(INCLUDE)/no_os_uart.h \ | ||
| $(INCLUDE)/no_os_pwm.h \ | ||
| $(INCLUDE)/no_os_dma.h \ |
There was a problem hiding this comment.
The NO_OS_INC_DIRS variable can be used to avoid adding all these explicitly.
The ADE7978, the ADE7933/ADE7932, and ADE7923 form a chipset dedicated to measuring 3-phase electrical energy using shunts as current sensors. The ADE7933/ADE7932 are isolated, sigma-delta analog-to-digital converters. The ADE7923 is a nonisolated, 3-channel Σ-Δ ADC. ADE7933 has 3 channels and ADE7932 2 channels. The ADE7978 is a high accuracy, 3-phase electrical energy measurement IC with serial interfaces and three flexible pulse outputs. This is a basic driver implementation with communication done on SPI. Signed-off-by: Radu Etz <[email protected]>
Example project for ADE7978 in chipset configuration with ADE7933/ADE7932, and ADE7923. Tested using the EVAL-ADE7978EBZ. The example includes measurments of the rms values. The example is tested with the AD-APARD32690-SL. Signed-off-by: Radu Etz <[email protected]>
|
v2 change log:
|
danmois
reviewed
Dec 23, 2024
| */ | ||
| struct measurements { | ||
| /* I rms value */ | ||
| float i_rms; |
There was a problem hiding this comment.
I wonder if it is a good idea to have float values in the driver.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pull Request Description
Basic driver for ADE7978 with SPI communication.
The ADE7978 communicates via a digital interface with ADE7933/ADE7932, or ADE7923 forming a
chipset dedicated to measuring 3-phase electrical energy using shunts as current sensors.
PR Type
PR Checklist