Skip to content

Conversation

hakehuang
Copy link
Contributor

  1. Create adc_power_shield sample for any board with adc can be used for power measurement.
  2. Add general_adc_platform power shiled, which can be used for power measurement with the sample application.
  3. the power data is stored in the power_shield folder for further analysis

@zephyrbot zephyrbot added area: ADC Analog-to-Digital Converter (ADC) platform: NXP NXP area: Twister Twister area: Samples Samples labels Aug 27, 2025
@hakehuang
Copy link
Contributor Author

log for mimxrt595_evk board

((.venv) ) ubuntu@ubuntu-OptiPlex-7050:/home/shared/disk/zephyr_project/zephyr_test/zephyr$ scripts/twister --device-testing --hardware-map /home/ubuntu/nxp/mimxrt595_evk_cm33/map.yaml -T samples/boards/nxp/mimxrt595_evk/system_off
ZEPHYR_BASE unset, using "/home/shared/disk/zephyr_project/zephyr_test/zephyr"
Renaming previous output directory to /home/shared/disk/zephyr_project/zephyr_test/zephyr/twister-out.1
INFO    - Using Ninja..
INFO    - Zephyr version: v4.2.0-2411-g0f6f916cbaa4
INFO    - Using 'zephyr' toolchain.
INFO    - Building initial testsuite list...
INFO    - Writing JSON report /home/shared/disk/zephyr_project/zephyr_test/zephyr/twister-out/testplan.json

Device testing on:

| Platform                      |        ID | Serial device                                         |
|-------------------------------|-----------|-------------------------------------------------------|
| mimxrt595_evk/mimxrt595s/cm33 | 727562924 | /dev/serial/by-id/usb-SEGGER_J-Link_000727562924-if00 |

INFO    - JOBS: 8
INFO    - Adding tasks to the queue...
INFO    - Added initial list of jobs to queue
INFO    - Total complete:    1/   1  100%  built (not run):    0, filtered:    0, failed:    0, error:    0
INFO    - 1 test scenarios (1 configurations) selected, 0 configurations filtered (0 by static filter, 0 at runtime).
INFO    - 1 of 1 executed test configurations passed (100.00%), 0 built (not run), 0 failed, 0 errored, with no warnings in 29.99 seconds.
INFO    - 1 of 1 executed test cases passed (100.00%) on 1 out of total 1154 platforms (0.09%).
INFO    - 1 test configurations executed on platforms, 0 test configurations were only built.


Copy link
Contributor

Choose a reason for hiding this comment

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

Please take a look at #94585, and only keep 1 platform per vendor.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks @JarmouniA PR #94585 looks good, I will update according after your PR merged

Copy link
Contributor

Choose a reason for hiding this comment

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

There is no need to wait for it to be merged, just apply the same filtering and only keep overlay/conf of 1 platform per vendor in the new sample.


config SEQUENCE_32BITS_REGISTERS
bool "ADC data sequences are on 32bits"
default n
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
default n

Comment on lines 36 to 37
pinctrl-0 = <&adc0_default>;
pinctrl-names = "default";
Copy link
Contributor

Choose a reason for hiding this comment

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

move above child node

@hakehuang hakehuang marked this pull request as draft August 28, 2025 03:32
@hakehuang hakehuang force-pushed the lc_power_shield branch 2 times, most recently from c19dcd6 to 78d25a9 Compare August 28, 2025 05:03
Copy link
Member

@dsseng dsseng left a comment

Choose a reason for hiding this comment

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

Thanks, this is a really great addition

Comment on lines 29 to 31
# Additional development tools
ipython>=8.0.0
jupyter>=1.0.0
Copy link
Member

Choose a reason for hiding this comment

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

Are these actually used here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

right, it is added by my local setting, will remove it

Comment on lines 8 to 14
# Set UTF-8 encoding for stdout/stderr
os.environ['PYTHONIOENCODING'] = 'utf-8'

# For Python 3.7+, you can also reconfigure stdout/stderr
if hasattr(sys.stdout, 'reconfigure'):
sys.stdout.reconfigure(encoding='utf-8')
sys.stderr.reconfigure(encoding='utf-8')
Copy link
Member

Choose a reason for hiding this comment

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

Move this under def main or __name__ == "__main__" so that the module is cleanly importable?

@hakehuang
Copy link
Contributor Author

@nordicjm, @dsseng I have updated according to your comments please help to review. Thanks

@nordicjm nordicjm dismissed their stale review September 3, 2025 06:38

DTS format fixed

@hakehuang
Copy link
Contributor Author

hakehuang commented Sep 4, 2025

I define a dft (device for test) which can also extend to multi-application build

Comment on lines +2 to +21
:name: Analog-to-Digital Converter (ADC) power shield sample
:relevant-api: adc_interface

Read analog inputs from ADC channels, using a sequence.

Overview
********

This sample is to enable general power shield for platfroms that have ADC support.


Building and Running
********************

Make sure that the ADC is enabled (``status = "okay";``) and has each channel as a
child node, with your desired settings like gain, reference, or acquisition time and
oversampling setting (if used). It is also needed to provide an alias ``adc0`` for the
desired adc. See :zephyr_file:`boards/nrf52840dk_nrf52840.overlay
<samples/drivers/adc/adc_dt/boards/nrf52840dk_nrf52840.overlay>` for an example of
such setup.
Copy link
Contributor

@kartben kartben Sep 4, 2025

Choose a reason for hiding this comment

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

All of this is very stale and copied from the original sample (related: do you really need to duplicate the 100s of overlays too??). I have to admit I don't really understand what this is trying to accomplish and the fact that the README hasn't been updated is not really helping.
Just a thought but, if you are calling this a "shield" then maybe you want to create an actual shield (or snippet?), and not a sample?

Copy link
Contributor Author

@hakehuang hakehuang Sep 5, 2025

Choose a reason for hiding this comment

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

@kartben, the sample application is modified from adc_sequence sample. The idea here is to create a general power shield, which means any adc featured zephyr platform can be used as a power shield. so we can take any of those 100s boards and flashed with this sample, you can use them as general_power_shield fixture for measuring power, see the https://github.com/zephyrproject-rtos/zephyr/blob/af6248f8b7509f505982b8fe5d6e9a6efd8bc0f5/scripts/pylib/power-twister-harness/README.rst this is a full picture for the usage.
the sample readme is just describe how the sample runs.

Copy link
Contributor Author

@hakehuang hakehuang Sep 5, 2025

Choose a reason for hiding this comment

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

I want to provide a platform agonic solution, such as any same feature Zephyr platform can function as the same.

image image the is a demo setup for general power measurement.

Copy link
Contributor

Choose a reason for hiding this comment

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

This comment thread still applies IMO. I am sorry if I am missing something but simply copying an existing code sample (and its dozens of overlays!) and then calling this a "shield" while shields are something completely different in Zephyr is really incredibly confusing.
Please don't ask for re-review when comments havne't been addressed - the README being an exact copy-paste of the original sample is also pretty problematic. Thanks!

@nashif nashif assigned nashif and unassigned kartben Sep 5, 2025
@hakehuang hakehuang requested a review from kartben September 26, 2025 02:47
@hakehuang
Copy link
Contributor Author

hakehuang commented Sep 26, 2025

@nashif please take a look. Thanks

@zephyrbot zephyrbot added area: Power Management platform: NXP MCU area: Tests Issues related to a particular existing or missing test labels Sep 29, 2025
enable power shield testing on mimxrt595_evk

```
scripts/twister --device-testing --device-serial /dev/ttyACM0
-T samples/boards/nxp/mimxrt595_evk/system_off
-X pm_probe:/dev/ttyACM1,115200
```

an power_shield folder will be created in the build path
whith measured voltage/current/power

Signed-off-by: Hake Huang <[email protected]>
add power_shield sample for general_adc_platform
power shiled, to record power/voltage/current

Signed-off-by: Hake Huang <[email protected]>
1. general power shield work with the adc_power_shield sample
2. config samples to config probe
3. add readme

Signed-off-by: Hake Huang <[email protected]>
enable power measure on power_mgmt_soc for
mimxrt1060_evk@C

Signed-off-by: Hake Huang <[email protected]>
Copy link

Copy link
Contributor

@bjarki-andreasen bjarki-andreasen left a comment

Choose a reason for hiding this comment

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

Has it been considered to just use the ADC shell and a "blank" application? Setting up and performing a reading can be done directly using the shell, no custom parsing of console input/output should be needed.

@hakehuang
Copy link
Contributor Author

hakehuang commented Oct 1, 2025

Has it been considered to just use the ADC shell and a "blank" application? Setting up and performing a reading can be done directly using the shell, no custom parsing of console input/output should be needed.

the main purpose of this harness is for CI. As we want to a unified test approach, if in a blank application, you will have to customize it by you own, in this case, you are not using CI. for example, you will not know which ADC channel is connect to which power rails, you have parse them according how you connect your ADC probes, and then you have to write your own script to do so. This is what I try to avoid. Please take a look at https://github.com/zephyrproject-rtos/zephyr/pull/95056/files#diff-9e48191a8fd4eac8e1fb6d4f1c73c35c3cdc65720c3eb6264964eb104e612534
you will need a config parser to link the shell output with your real probe settings, this is what I am trying to unify. @bjarki-andreasen, please be aware this is mainly for CI purpose.

@bjarki-andreasen
Copy link
Contributor

Has it been considered to just use the ADC shell and a "blank" application? Setting up and performing a reading can be done directly using the shell, no custom parsing of console input/output should be needed.

the main purpose of this harness is for CI. As we want to a unified test approach, if in a blank application, you will have to customize it by you own, in this case, you are not using CI. for example, you will not know which ADC channel is connect to which power rails, you have parse them according how you connect your ADC probes, and then you have to write your own script to do so. This is what I try to avoid. Please take a look at https://github.com/zephyrproject-rtos/zephyr/pull/95056/files#diff-9e48191a8fd4eac8e1fb6d4f1c73c35c3cdc65720c3eb6264964eb104e612534 you will need a config parser to link the shell output with your real probe settings, this is what I am trying to unify. @bjarki-andreasen, please be aware this is mainly for CI purpose.

We already have a twister harness for the shell, can it not be reused? and regarding the specific ADC channel, that can be added in an overlay when building surely, like this PR is already doing? We can also add a custom shell command to use specifically if needed, no need to implement command line parsing in the sample

@hakehuang
Copy link
Contributor Author

hakehuang commented Oct 3, 2025

We already have a twister harness for the shell, can it not be reused? and regarding the specific ADC channel, that can be added in an overlay when building surely, like this PR is already doing? We can also add a custom shell command to use specifically if needed, no need to implement command line parsing in the sample

@bjarki-andreasen if you look at my PR, I add a 'dft' which reuse the whole pytest serial handler see

def dft(request: pytest.FixtureRequest, dft_object) -> Generator[DeviceAdapter, None, None]:
. ADC channel is already controlled by overlay, but the problem is that we need map the channel to the target power rails see below definition:
the channels_p and channel_n to the power rail "VDD_CORE" / "VDD_IO" is by your board setting definition, this could be different pre your test harness setting, and this has to be customized, here is use a yaml to control it.

device_id: "adc_power_monitor_01"
routes:
  - id: 0
    name: "VDD_CORE"
    shunt_resistor: 0.1  # ohms
    gain: 1.0
    type: "single"
    channels:
      channels_p: 4
      channels_n: 0
  - id: 1
    name: "VDD_IO"
    shunt_resistor: 0.1  # ohms
    gain: 1.0
    type: "single"
    channels:
      channels_p: 7
      channels_n: 3
calibration:
  offset: 0.0
  scale: 1.0

so nowhat where to add, we either add shell command or use a dedicated power shield sample application, we have to parse the shell/command output to get the board settings on its probe capacities, see below.

{
  "channel_count": 2,
  "resolution": 12,
  "channels": {
    "0": {
      "mode": "single",
      "verf_mv": 3300
    },
    "3": {
      "mode": "single",
      "verf_mv": 3300
    },
    "4": {
      "mode": "single",
      "verf_mv": 3300
    },
    "7": {
      "mode": "single",
      "verf_mv": 3300
    }
  }
}

and we need map the probe capacities to the target measure power rails as I mentioned above. My understanding is that you propose to use a series of shell command to replace the power measure sample which will be running on power shield. this will add adc shell linked with power calculations I am not sure this will accepted by ADC shell owner? As in this PR, we have an independent sample to host everything, move those to shell command needs approval, can you help to confirm on my understanding and if accepted by ADC shell group I can move the implement to shell.

@bjarki-andreasen
Copy link
Contributor

We already have a twister harness for the shell, can it not be reused? and regarding the specific ADC channel, that can be added in an overlay when building surely, like this PR is already doing? We can also add a custom shell command to use specifically if needed, no need to implement command line parsing in the sample

@bjarki-andreasen if you look at my PR, I add a 'dft' which reuse the whole pytest serial handler see

def dft(request: pytest.FixtureRequest, dft_object) -> Generator[DeviceAdapter, None, None]:
. ADC channel is already controlled by overlay, but the problem is that we need map the channel to the target power rails see below definition:
the channels_p and channel_n to the power rail "VDD_CORE" / "VDD_IO" is by your board setting definition, this could be different pre your test harness setting, and this has to be customized, here is use a yaml to control it.

device_id: "adc_power_monitor_01"
routes:
  - id: 0
    name: "VDD_CORE"
    shunt_resistor: 0.1  # ohms
    gain: 1.0
    type: "single"
    channels:
      channels_p: 4
      channels_n: 0
  - id: 1
    name: "VDD_IO"
    shunt_resistor: 0.1  # ohms
    gain: 1.0
    type: "single"
    channels:
      channels_p: 7
      channels_n: 3
calibration:
  offset: 0.0
  scale: 1.0

so nowhat where to add, we either add shell command or use a dedicated power shield sample application, we have to parse the shell/command output to get the board settings on its probe capacities, see below.

{
  "channel_count": 2,
  "resolution": 12,
  "channels": {
    "0": {
      "mode": "single",
      "verf_mv": 3300
    },
    "3": {
      "mode": "single",
      "verf_mv": 3300
    },
    "4": {
      "mode": "single",
      "verf_mv": 3300
    },
    "7": {
      "mode": "single",
      "verf_mv": 3300
    }
  }
}

and we need map the probe capacities to the target measure power rails as I mentioned above. My understanding is that you propose to use a series of shell command to replace the power measure sample which will be running on power shield. this will add adc shell linked with power calculations I am not sure this will accepted by ADC shell owner? As in this PR, we have an independent sample to host everything, move those to shell command needs approval, can you help to confirm on my understanding and if accepted by ADC shell group I can move the implement to shell.

No changes to the adc shell is needed. We would just create a set of custom shell commands as part of the power shield sample. The issue is not with having a sample at all, we need that, but with how the serial communication is implemented. We can freely create sample specific shell commands, so why not use that?

@hakehuang
Copy link
Contributor Author

No changes to the adc shell is needed. We would just create a set of custom shell commands as part of the power shield sample. The issue is not with having a sample at all, we need that, but with how the serial communication is implemented. We can freely create sample specific shell commands, so why not use that?

ok, I see, I will update this PR to add a sample specific shell command. but I have some concerns on the code size, I enable the stm32f030dlk as the low cost shield, which code size is very critical. Let me try and feedback.

@hakehuang hakehuang marked this pull request as draft October 5, 2025 01:57
@bjarki-andreasen
Copy link
Contributor

No changes to the adc shell is needed. We would just create a set of custom shell commands as part of the power shield sample. The issue is not with having a sample at all, we need that, but with how the serial communication is implemented. We can freely create sample specific shell commands, so why not use that?

ok, I see, I will update this PR to add a sample specific shell command. but I have some concerns on the code size, I enable the stm32f030dlk as the low cost shield, which code size is very critical. Let me try and feedback.

For this, enable SHELL_MINIMAL, the entire shell should "only" take up 6-8K ROM, if that is too much we may have to go with a custom solution

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: ADC Analog-to-Digital Converter (ADC) area: Power Management area: Samples Samples area: Tests Issues related to a particular existing or missing test area: Twister Twister platform: NXP MCU platform: NXP NXP
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants