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

i2c: add i2c shell with scan cmd #21797

Merged
merged 2 commits into from
Jan 10, 2020

Conversation

nashif
Copy link
Member

@nashif nashif commented Jan 9, 2020

Add a shell to interact with I2C devices. Currently scanning for devices is
supported. Example output:


uart:~$ i2c scan I2C_2
     0  1  2  3  4  5  6  7  8  9  a  b  c  d  e  f
00:             -- -- -- -- -- -- -- -- -- -- -- --
10: -- -- -- -- -- -- -- -- -- -- -- -- -- -- 1e --
20: -- -- -- -- -- -- -- -- -- 29 -- -- -- 2d -- --
30: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
40: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
50: -- -- -- -- -- -- 56 -- -- -- -- -- -- 5d -- 5f
60: -- -- -- -- -- -- -- -- -- -- 6a -- -- -- -- --
70: -- -- -- -- -- -- -- --
7 devices found on I2C_2

This replaces a dedicated sample that did the same thing.

@zephyrbot
Copy link
Collaborator

zephyrbot commented Jan 9, 2020

All checks are passing now.

Tip: The bot edits this comment instead of posting a new one, so you can check the comment's history to see earlier messages.


#define LOG_LEVEL CONFIG_LOG_DEFAULT_LEVEL
#include <logging/log.h>
LOG_MODULE_REGISTER(i2c_shell);
Copy link
Collaborator

Choose a reason for hiding this comment

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

can be shorten to LOG_MODULE_REGISTER(i2c_shell, CONFIG_LOG_DEFAULT_LEVEL);

}

SHELL_STATIC_SUBCMD_SET_CREATE(sub_i2c_cmds,
/* Alphabetically sorted. */
Copy link
Contributor

Choose a reason for hiding this comment

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

As far as I remember you do no need to sort sub-commands any more

Copy link
Member Author

Choose a reason for hiding this comment

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

copy pasted from another shell :)

);

SHELL_STATIC_SUBCMD_SET_CREATE(sub_i2c,
#ifdef CONFIG_I2C_0
Copy link
Contributor

Choose a reason for hiding this comment

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

This is good approach.
In future we can think of implementing this as a dynamic commands. All active I2C instances can be collected in runtime.

Copy link
Member Author

Choose a reason for hiding this comment

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

tried to do that similar to how it was done in the sensor shell, but something did not work for me, so I reverted to this.

Copy link
Member Author

Choose a reason for hiding this comment

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

changed and implemented dynamic device detection...

@nashif nashif force-pushed the i2c_shell_scan branch 2 times, most recently from 687430b to 718c362 Compare January 9, 2020 19:01
@nashif nashif closed this Jan 9, 2020
@nashif nashif reopened this Jan 9, 2020
entry->help = NULL;
entry->subcmd = &dsub_device_name;

for (dev = __device_init_start; dev != __device_init_end; dev++) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

ha ha, that is the same as device_get_by_prefix

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, we definitely need something like that in some form, I see this code being duplicated all over the place.

Copy link
Member Author

Choose a reason for hiding this comment

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

For example, it would be great to have the "on-bus" from dts somewhere in the device structure so we can filter based on a bus.

Copy link
Collaborator

Choose a reason for hiding this comment

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

probably here but in general a subsystem/app like lvgl, cfb, netwok... does not care what bus it is.

I would like to see the approach in #21709 accepted as long #20253 is WIP. See also my comment there

drivers/i2c/i2c_shell.c Outdated Show resolved Hide resolved
drivers/i2c/i2c_shell.c Show resolved Hide resolved
drivers/i2c/i2c_shell.c Show resolved Hide resolved
Now part of the I2C shell and works for all boards.

Signed-off-by: Anas Nashif <anas.nashif@intel.com>
Add a shell to interact with I2C devices. Currently scanning for devices
is supported. Example output:

uart:~$ i2c scan I2C_1
     0  1  2  3  4  5  6  7  8  9  a  b  c  d  e  f
00:             -- -- -- -- -- -- -- -- -- -- -- --
10: -- -- -- -- -- -- -- -- 18 -- -- -- -- -- -- --
20: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
30: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
40: 40 -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
50: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
60: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
70: -- -- -- -- -- -- -- --
2 devices found on I2C_1
uart:~$ i2c scan I2C_3
i2c - I2C commands
Subcommands:
  I2C_1  :I2C_1
  I2C_2  :I2C_2
uart:~$ i2c scan I2C_2
     0  1  2  3  4  5  6  7  8  9  a  b  c  d  e  f
00:             -- -- -- -- -- -- -- -- -- -- -- --
10: -- -- -- -- -- -- -- -- -- -- -- -- -- -- 1e --
20: -- -- -- -- -- -- -- -- -- 29 -- -- -- 2d -- --
30: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
40: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
50: -- -- -- -- -- -- 56 -- -- -- -- -- -- 5d -- 5f
60: -- -- -- -- -- -- -- -- -- -- 6a -- -- -- -- --
70: -- -- -- -- -- -- -- --
7 devices found on I2C_2

This shell is based on the a sample that did the same thing and was
limited in support. The sample is now removed in favour of this generic
shell module.

Signed-off-by: Anas Nashif <anas.nashif@intel.com>
@nashif nashif merged commit 8900543 into zephyrproject-rtos:master Jan 10, 2020
@nashif nashif deleted the i2c_shell_scan branch January 10, 2020 13:55
@pabigot
Copy link
Collaborator

pabigot commented Jan 17, 2020

Wonderful. Presumably this is samples/subsys/shell/shell_module? Is this supposed to work?

uart:~$ i2c scan I2C_0
i2c: command not found

For anybody else just trying to get their job done, options are revert 7c9a87a locally, or (for Nordic at least) add these to the prj.conf for the shell module sample:

CONFIG_I2C=y
CONFIG_I2C_NRFX=y

Then ignore the cascade of errors that show up for all the addresses that don't have a device on it.

Reverting is a lot easier.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants