Hi Paul,
thank you for this patch!
> This makes it easier for new users to understand what is going on when
> they have a problem listing i2c busses that they do not understand.
I like this basically. I do have questions, though.
> @@ -199,6 +218,13 @@ struct i2c_adap *gather_i2c_busses(void)
> /* look in sysfs */
> /* First figure out where sysfs was mounted */
> if ((f = fopen("/proc/mounts", "r")) == NULL) {
> + fprintf(stderr, "Error: Could not open /proc/mounts: "
> + "%s\n", strerror(errno));
> + if (errno == ENOENT) {
> + fprintf(stderr, "Please mount procfs: "
> + "%smount -t procfs proc /proc\n",
> + getenv("SUDO_COMMAND") ? "sudo " : "");
Hmm, I wonder if a simple "Is it mounted?" isn't enough?
> + fprintf(stderr, "Error: Could not find sysfs mount\n");
> + fprintf(stderr, "Please mount sysfs: "
> + "%smount -t sysfs sysfs /sys\n",
> + getenv("SUDO_COMMAND") ? "sudo " : "");
Ditto.
> goto done;
> }
>
> /* Bus numbers in i2c-adapter don't necessarily match those in
> i2c-dev and what we really care about are the i2c-dev numbers.
> Unfortunately the names are harder to get in i2c-dev */
> + sysfs_end = strlen(sysfs);
> strcat(sysfs, "/class/i2c-dev");
> - if(!(dir = opendir(sysfs)))
> + if (!(dir = opendir(sysfs))) {
> + if (errno == ENOENT) {
> + /* Check if there are i2c bus devices in other dirs
> + as when there are none the error isn't useful
> + as loading i2c-dev also won't find devices */
> + int devices_present = 0;
> + strcpy(sysfs + sysfs_end, "/bus/i2c/devices");
> + devices_present = dir_has_entries(sysfs);
> + if (! devices_present) {
> + strcpy(sysfs + sysfs_end, "/class/i2c-adapter");
> + devices_present = dir_has_entries(sysfs);
> + }
> + if (devices_present) {
> + fprintf(stderr, "Error: Could not find dir "
> + "`%s`\n", sysfs);
> + fprintf(stderr, "Please load i2c-dev: "
> + "%smodprobe i2c-dev\n",
> + getenv("SUDO_COMMAND") ? "sudo " : "");
> + }
If there are no devices_present here, then we fail without printing
something? Is this intended?
> + } else {
> + fprintf(stderr, "Error: Could not open dir "
> + "`%s': %s\n", sysfs, strerror(errno));
Despite the above detail, I think this adds quite some code (also
counting dir_has_entries). Since I think we need i2c-dev anyway, can't
we just do:
1) say "please load i2c-dev" if it is not loaded
2) say "could not open dir" if it is loaded
Yes, for rare cases this could mean that loading "i2c-dev" does not
solve the problem, but using i2ctools without i2c-dev is going to fail
at some point anyhow, so IMHO we can complain about this early?
Makes sense? Did I miss something?
Happy hacking,
Wolfram
Attachment:
signature.asc
Description: PGP signature
