[cpp] Driver names for color, infrared, and ultrasound sensors have changed

[ddemidov]

In the latest kernel driver names for color, infrared, and ultrasonic sensors have apparently changed. See ddemidov/ev3dev-lang-python#2.

[WasabiFan]

@ddemidov Thanks for catching that. The C++ bindings haven't been updated in a while, so there could be more of these issues that you find.

@fdetro has a branch open right now that may have some of these fixes (that was where the C++ binding was being transitioned to the autogen system). @ddemidov If you have some spare time, would you be willing to continue that transition?

[ddemidov]

I think I could pick up the transition process. I did not pay much attention to the implementation yet, but it seems that switching to auto-generation system would require some API changes in the library. @WasabiFan, @fdetro, is that ok with you?

Also, I understand that individual bindings are gradually moving to git submodules. Would this be a good time to do this for cpp?

[WasabiFan]

@ddemidov That's O.K. with me, although I am not sure if you will need to change it very much (the job is already at least partially done in that extra branch).

Also, I understand that individual bindings are gradually moving to git submodules. Would this be a good time to do this for cpp?

We were separating the bindings so that the ones that weren't maintained centrally could be 'owned' by their respective owners, and they could have full control. Honestly, I think it's up to you and @fdetro.

Thanks for looking into this!

P.S. If you do start working on this and run in to issues with our generation script, I'd be happy to help you out.

[ddemidov]

OK, I'll start working on this in my spare time.

[ddemidov]

I'll close this, since autogen should take care of the name change. I've started the work on cpp bindings migration in my own fork.

[fdetro]

@ddemidov Thank you for finishing my work on the autogen stuff and merging it to the develop branch. I was very busy with other stuff during the last months and plan to update my ev3dev setup to the latest state today.

[robojay]

Not sure if you caught this already, but the NXT ultrasonic name has also changed (lego-nxt-us). I don't see it updated in your merge.

[ddemidov]

@robojay, thank you for the catch! I don't have any NXT hardware, so could you please confirm that 2d3eba4 works for you?

[ddemidov]

In fact, I am not sure nxt_ultrasonic constant is used in the code at all. Can you even use the sensor with the current ev3dev.{cpp,h}?

[robojay]

I'm in the process of setting up a build environment and (hopefully) will be able to try things out soon. I was working from the top level down... tried to access the NXT ultrasonic sensor via Python, and ended up in the c++ code.

[ddemidov]

I can provide you with a python egg that is based on the commit if you like.

[robojay]

That would be great.

[ddemidov]

You can get it at https://www.dropbox.com/s/h0jemd98tbznuzn/python_ev3dev-0.1.0.post2-py2.7-linux-armv5tejl.egg?dl=0

[robojay]

Doesn't appear to be working, but I could be doing the install improperly. Should I just be able to put the .egg on my EV3, and on the EV3 do: sudo easy_install (...local egg file...) ? Or, do I need to somehow uninstall prior versions?

[ddemidov]

That is the correct installation method. You should be able to check if the package has been installed successfully by

>>> from ev3dev import *
>>> sensor.nxt_ultrasonic
'lego-nxt-us'

But as I said, I don't see where in the C++ code the constant is used. So it could be the nxt_ultrasonic sensor is just not supported at all by the C++ bindings. I think it should be very easy to add the support by copying the ultrasonic_sensor class. But since I don't have the sensor, I won't be able to test the change. Do you think you could do this?

[ddemidov]

Since the sensor::sensor constructor takes a set of strings for driver names, the change could be as simple as

diff --git a/cpp/ev3dev.cpp b/cpp/ev3dev.cpp
index 601f786..9956d50 100644
--- a/cpp/ev3dev.cpp
+++ b/cpp/ev3dev.cpp
@@ -563,7 +563,7 @@ const mode_type ultrasonic_sensor::mode_single_cm { "US-SI-CM"   };
 const mode_type ultrasonic_sensor::mode_single_in { "US-SI-IN"   };

 ultrasonic_sensor::ultrasonic_sensor(port_type port_) :
-  sensor(port_, { ev3_ultrasonic })
+  sensor(port_, { ev3_ultrasonic, nxt_ultrasonic })
 {
 }

[robojay]

Ok, I'm running the right code, and it isn't detected.

Yes, I can give it a try once my EV3 build environment is up and running, unless there is a simpler way to test (?).

[robojay]

I was just looking at the c++ code. It doesn't look like any of the NXT sensors are being handled?

[ddemidov]

It doesn't look like any of the NXT sensors are being handled?

It does look like this.

I have updated the egg on dropbox (https://www.dropbox.com/s/h0jemd98tbznuzn/python_ev3dev-0.1.0.post2-py2.7-linux-armv5tejl.egg?dl=0) to include the change I proposed above. Can you check if the sensor is detected now? If it does work, may be we could apply the same cure to the rest of the nxt devices.

[robojay]

Here are some details. What do you think about the different port_name formats being returned for the NXT sensors versus the EV3 one? I'm not great with c++, but it looks like that might be a problem too.

>>> from ev3dev import *
>>> sensor.nxt_ultrasonic
'lego-nxt-us'
>>> s = sensor(INPUT_1)
>>> s.connected
False
>>> s = sensor(INPUT_2)
>>> s.connected
True
>>> s.driver_name
'lego-ev3-uart-30'
>>> s = sensor(INPUT_3)
>>> s.connected
False
>>> exit()
jay@ev3dev:~/py/testing$ ls /sys/class/lego-sensor/
sensor0  sensor1  sensor4
jay@ev3dev:~/py/testing$ cat /sys/class/lego-sensor/sensor0/port_name 
in1:i2c1
jay@ev3dev:~/py/testing$ cat /sys/class/lego-sensor/sensor0/driver_name 
lego-nxt-us
jay@ev3dev:~/py/testing$ cat /sys/class/lego-sensor/sensor1/port_name 
in3:i2c1
jay@ev3dev:~/py/testing$ cat /sys/class/lego-sensor/sensor1/driver_name 
lego-nxt-us
jay@ev3dev:~/py/testing$ cat /sys/class/lego-sensor/sensor4/port_name 
in2
jay@ev3dev:~/py/testing$ cat /sys/class/lego-sensor/sensor4/driver_name 
lego-ev3-uart-30

[robojay]

Oh, and no, it didn't work...

[robojay]

Aha!

>>> s = sensor('in1:i2c1')
>>> s.connected
True
>>> s.driver_name
'lego-nxt-us'

[ddemidov]

I was just writing a comment suggesting that :).

[robojay]

This works too...

>>> s = ultrasonic_sensor('in1:i2c1')
>>> s.connected
True

[ddemidov]

Do you also get the distance readings correctly?

[robojay]

It appears to, at least for single reading mode.

>>> s.mode = 'US-SI-CM'
>>> s.value()
10
>>> s.mode = 'US-SI-CM'
>>> s.value()
22

[ddemidov]

I'll propose a pull request with the change then. Do you have other NXT devices to test?

[robojay]

Great! -- Yes, I've got them all, and can check them out.

[robojay]

Thanks for working through this :-)

[ddemidov]

Thanks for testing!

[robojay]

Does the last egg you provided just fix the ultrasonic, or all the NXT sensors?

[ddemidov]

It just fixes the ultrasonic sensor. May be we should open a new issue for the rest of the NXT devices.

[robojay]

I just noticed there is no mention of the LEGO NXT color sensor being supported in the "supported sensors" list, just the older light sensor. Not sure how similar it is to the EV3 color sensor... Anyways, I can test any of the LEGO NXT sensors (except temperature) when ready.