[PATCH] iio: srf04: fix wrong limitation in distance measuring

[PATCH] iio: srf04: fix wrong limitation in distance measuring

[Andreas Klinger]

The measured time value in the driver is limited to the maximum distance
which can be read by the sensor. This limitation was wrong and is fixed
by this patch.

It also takes into account that we are supporting a variety of sensors
today and that the recently added sensors have a higher maximum
distance range.

Suggested-by: Zbyněk Kocur <zbynek.kocur@fel.cvut.cz>
Signed-off-by: Andreas Klinger <ak@it-klinger.de>
---
 drivers/iio/proximity/srf04.c | 29 +++++++++++++++--------------
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/drivers/iio/proximity/srf04.c b/drivers/iio/proximity/srf04.c
index 8b50d56b0a03..01eb8cc63076 100644
--- a/drivers/iio/proximity/srf04.c
+++ b/drivers/iio/proximity/srf04.c
@@ -110,7 +110,7 @@ static int srf04_read(struct srf04_data *data)
 	udelay(data->cfg->trigger_pulse_us);
 	gpiod_set_value(data->gpiod_trig, 0);
 
-	/* it cannot take more than 20 ms */
+	/* it should not take more than 20 ms until echo is rising */
 	ret = wait_for_completion_killable_timeout(&data->rising, HZ/50);
 	if (ret < 0) {
 		mutex_unlock(&data->lock);
@@ -120,7 +120,8 @@ static int srf04_read(struct srf04_data *data)
 		return -ETIMEDOUT;
 	}
 
-	ret = wait_for_completion_killable_timeout(&data->falling, HZ/50);
+	/* it cannot take more than 50 ms until echo is falling */
+	ret = wait_for_completion_killable_timeout(&data->falling, HZ/20);
 	if (ret < 0) {
 		mutex_unlock(&data->lock);
 		return ret;
@@ -135,19 +136,19 @@ static int srf04_read(struct srf04_data *data)
 
 	dt_ns = ktime_to_ns(ktime_dt);
 	/*
-	 * measuring more than 3 meters is beyond the capabilities of
-	 * the sensor
+	 * measuring more than 6,45 meters is beyond the capabilities of
+	 * the supported sensors
 	 * ==> filter out invalid results for not measuring echos of
 	 *     another us sensor
 	 *
 	 * formula:
-	 *         distance       3 m
-	 * time = ---------- = --------- = 9404389 ns
-	 *          speed       319 m/s
+	 *         distance     6,45 * 2 m
+	 * time = ---------- = ------------ = 40438871 ns
+	 *          speed         319 m/s
 	 *
 	 * using a minimum speed at -20 °C of 319 m/s
 	 */
-	if (dt_ns > 9404389)
+	if (dt_ns > 40438871)
 		return -EIO;
 
 	time_ns = dt_ns;
@@ -159,20 +160,20 @@ static int srf04_read(struct srf04_data *data)
 	 *   with Temp in °C
 	 *   and speed in m/s
 	 *
-	 * use 343 m/s as ultrasonic speed at 20 °C here in absence of the
+	 * use 343,5 m/s as ultrasonic speed at 20 °C here in absence of the
 	 * temperature
 	 *
 	 * therefore:
-	 *             time     343
-	 * distance = ------ * -----
-	 *             10^6       2
+	 *             time     343,5     time * 106
+	 * distance = ------ * ------- = ------------
+	 *             10^6         2         617176
 	 *   with time in ns
 	 *   and distance in mm (one way)
 	 *
-	 * because we limit to 3 meters the multiplication with 343 just
+	 * because we limit to 6,45 meters the multiplication with 106 just
 	 * fits into 32 bit
 	 */
-	distance_mm = time_ns * 343 / 2000000;
+	distance_mm = time_ns * 106 / 617176;
 
 	return distance_mm;
 }
-- 
2.11.0

Re: [PATCH] iio: srf04: fix wrong limitation in distance measuring

[Jonathan Cameron]

On Sun, 23 Jun 2019 14:29:10 +0200
Andreas Klinger <ak@it-klinger.de> wrote:

> The measured time value in the driver is limited to the maximum distance
> which can be read by the sensor. This limitation was wrong and is fixed
> by this patch.
> 
> It also takes into account that we are supporting a variety of sensors
> today and that the recently added sensors have a higher maximum
> distance range.
> 
> Suggested-by: Zbyněk Kocur <zbynek.kocur@fel.cvut.cz>
> Signed-off-by: Andreas Klinger <ak@it-klinger.de>
Ideally I'm looking for Zbyněk to confirm that this addresses the
original question.

Thanks,

Jonathan

> ---
>  drivers/iio/proximity/srf04.c | 29 +++++++++++++++--------------
>  1 file changed, 15 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/iio/proximity/srf04.c b/drivers/iio/proximity/srf04.c
> index 8b50d56b0a03..01eb8cc63076 100644
> --- a/drivers/iio/proximity/srf04.c
> +++ b/drivers/iio/proximity/srf04.c
> @@ -110,7 +110,7 @@ static int srf04_read(struct srf04_data *data)
>  	udelay(data->cfg->trigger_pulse_us);
>  	gpiod_set_value(data->gpiod_trig, 0);
>  
> -	/* it cannot take more than 20 ms */
> +	/* it should not take more than 20 ms until echo is rising */
>  	ret = wait_for_completion_killable_timeout(&data->rising, HZ/50);
>  	if (ret < 0) {
>  		mutex_unlock(&data->lock);
> @@ -120,7 +120,8 @@ static int srf04_read(struct srf04_data *data)
>  		return -ETIMEDOUT;
>  	}
>  
> -	ret = wait_for_completion_killable_timeout(&data->falling, HZ/50);
> +	/* it cannot take more than 50 ms until echo is falling */
> +	ret = wait_for_completion_killable_timeout(&data->falling, HZ/20);
>  	if (ret < 0) {
>  		mutex_unlock(&data->lock);
>  		return ret;
> @@ -135,19 +136,19 @@ static int srf04_read(struct srf04_data *data)
>  
>  	dt_ns = ktime_to_ns(ktime_dt);
>  	/*
> -	 * measuring more than 3 meters is beyond the capabilities of
> -	 * the sensor
> +	 * measuring more than 6,45 meters is beyond the capabilities of
> +	 * the supported sensors
>  	 * ==> filter out invalid results for not measuring echos of
>  	 *     another us sensor
>  	 *
>  	 * formula:
> -	 *         distance       3 m
> -	 * time = ---------- = --------- = 9404389 ns
> -	 *          speed       319 m/s
> +	 *         distance     6,45 * 2 m
> +	 * time = ---------- = ------------ = 40438871 ns
> +	 *          speed         319 m/s
>  	 *
>  	 * using a minimum speed at -20 °C of 319 m/s
>  	 */
> -	if (dt_ns > 9404389)
> +	if (dt_ns > 40438871)
>  		return -EIO;
>  
>  	time_ns = dt_ns;
> @@ -159,20 +160,20 @@ static int srf04_read(struct srf04_data *data)
>  	 *   with Temp in °C
>  	 *   and speed in m/s
>  	 *
> -	 * use 343 m/s as ultrasonic speed at 20 °C here in absence of the
> +	 * use 343,5 m/s as ultrasonic speed at 20 °C here in absence of the
>  	 * temperature
>  	 *
>  	 * therefore:
> -	 *             time     343
> -	 * distance = ------ * -----
> -	 *             10^6       2
> +	 *             time     343,5     time * 106
> +	 * distance = ------ * ------- = ------------
> +	 *             10^6         2         617176
>  	 *   with time in ns
>  	 *   and distance in mm (one way)
>  	 *
> -	 * because we limit to 3 meters the multiplication with 343 just
> +	 * because we limit to 6,45 meters the multiplication with 106 just
>  	 * fits into 32 bit
>  	 */
> -	distance_mm = time_ns * 343 / 2000000;
> +	distance_mm = time_ns * 106 / 617176;
>  
>  	return distance_mm;
>  }

Re: [PATCH] iio: srf04: fix wrong limitation in distance measuring

[Jonathan Cameron]

On Wed, 26 Jun 2019 20:59:08 +0200
Zbyněk Kocur <zbynek.kocur@fel.cvut.cz> wrote:

> Hello, Andreas,
> 
> Your patch seems to solve my question. I haven't had time to test it because of holidays. I'll test it as soon as possible.

No rush, it's a clear fix anyway so we can queue this up
in the rc phase of the next kernel cycle!

Thanks,

Jonathan

> 
> Zbyněk
> ---
> email: zbynek.kocur@fel.cvut.cz
> phone: +420 224 354 054
> web: http://www.fel.cvut.cz
> Department of Telecommunications Engineering
> Faculty of Electrical Engineering
> 
> > On 26 Jun 2019, at 20:21, Jonathan Cameron <jic23@kernel.org> wrote:
> > 
> > On Sun, 23 Jun 2019 14:29:10 +0200
> > Andreas Klinger <ak@it-klinger.de> wrote:
> >   
> >> The measured time value in the driver is limited to the maximum distance
> >> which can be read by the sensor. This limitation was wrong and is fixed
> >> by this patch.
> >> 
> >> It also takes into account that we are supporting a variety of sensors
> >> today and that the recently added sensors have a higher maximum
> >> distance range.
> >> 
> >> Suggested-by: Zbyněk Kocur <zbynek.kocur@fel.cvut.cz>
> >> Signed-off-by: Andreas Klinger <ak@it-klinger.de>  
> > Ideally I'm looking for Zbyněk to confirm that this addresses the
> > original question.
> > 
> > Thanks,
> > 
> > Jonathan
> >   
> >> ---
> >> drivers/iio/proximity/srf04.c | 29 +++++++++++++++--------------
> >> 1 file changed, 15 insertions(+), 14 deletions(-)
> >> 
> >> diff --git a/drivers/iio/proximity/srf04.c b/drivers/iio/proximity/srf04.c
> >> index 8b50d56b0a03..01eb8cc63076 100644
> >> --- a/drivers/iio/proximity/srf04.c
> >> +++ b/drivers/iio/proximity/srf04.c
> >> @@ -110,7 +110,7 @@ static int srf04_read(struct srf04_data *data)
> >> 	udelay(data->cfg->trigger_pulse_us);
> >> 	gpiod_set_value(data->gpiod_trig, 0);
> >> 
> >> -	/* it cannot take more than 20 ms */
> >> +	/* it should not take more than 20 ms until echo is rising */
> >> 	ret = wait_for_completion_killable_timeout(&data->rising, HZ/50);
> >> 	if (ret < 0) {
> >> 		mutex_unlock(&data->lock);
> >> @@ -120,7 +120,8 @@ static int srf04_read(struct srf04_data *data)
> >> 		return -ETIMEDOUT;
> >> 	}
> >> 
> >> -	ret = wait_for_completion_killable_timeout(&data->falling, HZ/50);
> >> +	/* it cannot take more than 50 ms until echo is falling */
> >> +	ret = wait_for_completion_killable_timeout(&data->falling, HZ/20);
> >> 	if (ret < 0) {
> >> 		mutex_unlock(&data->lock);
> >> 		return ret;
> >> @@ -135,19 +136,19 @@ static int srf04_read(struct srf04_data *data)
> >> 
> >> 	dt_ns = ktime_to_ns(ktime_dt);
> >> 	/*
> >> -	 * measuring more than 3 meters is beyond the capabilities of
> >> -	 * the sensor
> >> +	 * measuring more than 6,45 meters is beyond the capabilities of
> >> +	 * the supported sensors
> >> 	 * ==> filter out invalid results for not measuring echos of
> >> 	 *     another us sensor
> >> 	 *
> >> 	 * formula:
> >> -	 *         distance       3 m
> >> -	 * time = ---------- = --------- = 9404389 ns
> >> -	 *          speed       319 m/s
> >> +	 *         distance     6,45 * 2 m
> >> +	 * time = ---------- = ------------ = 40438871 ns
> >> +	 *          speed         319 m/s
> >> 	 *
> >> 	 * using a minimum speed at -20 °C of 319 m/s
> >> 	 */
> >> -	if (dt_ns > 9404389)
> >> +	if (dt_ns > 40438871)
> >> 		return -EIO;
> >> 
> >> 	time_ns = dt_ns;
> >> @@ -159,20 +160,20 @@ static int srf04_read(struct srf04_data *data)
> >> 	 *   with Temp in °C
> >> 	 *   and speed in m/s
> >> 	 *
> >> -	 * use 343 m/s as ultrasonic speed at 20 °C here in absence of the
> >> +	 * use 343,5 m/s as ultrasonic speed at 20 °C here in absence of the
> >> 	 * temperature
> >> 	 *
> >> 	 * therefore:
> >> -	 *             time     343
> >> -	 * distance = ------ * -----
> >> -	 *             10^6       2
> >> +	 *             time     343,5     time * 106
> >> +	 * distance = ------ * ------- = ------------
> >> +	 *             10^6         2         617176
> >> 	 *   with time in ns
> >> 	 *   and distance in mm (one way)
> >> 	 *
> >> -	 * because we limit to 3 meters the multiplication with 343 just
> >> +	 * because we limit to 6,45 meters the multiplication with 106 just
> >> 	 * fits into 32 bit
> >> 	 */
> >> -	distance_mm = time_ns * 343 / 2000000;
> >> +	distance_mm = time_ns * 106 / 617176;
> >> 
> >> 	return distance_mm;
> >> }  
> >   
> 

Re: [PATCH] iio: srf04: fix wrong limitation in distance measuring

[Zbyněk Kocur]

Thank you, i will write a feedback after testing.


Zbynek
---
email: zbynek.kocur@fel.cvut.cz
phone: +420 224 354 054
web: http://www.fel.cvut.cz
Department of Telecommunications Engineering
Faculty of Electrical Engineering

> On 26 Jun 2019, at 21:49, Jonathan Cameron <jic23@kernel.org> wrote:
> 
> No rush, it's a clear fix anyway so we can queue this up
> in the rc phase of the next kernel cycle!

Re: [PATCH] iio: srf04: fix wrong limitation in distance measuring

[Andreas Klinger]

Hi Zbynek,

do you have any feedback for us?

Thank you,

Andreas


Zbyněk Kocur <zbynek.kocur@fel.cvut.cz> schrieb am Mi, 26. Jun 21:50:
> Thank you, i will write a feedback after testing.
> 
> 
> Zbynek
> ---
> email: zbynek.kocur@fel.cvut.cz
> phone: +420 224 354 054
> web: http://www.fel.cvut.cz
> Department of Telecommunications Engineering
> Faculty of Electrical Engineering
> 
> > On 26 Jun 2019, at 21:49, Jonathan Cameron <jic23@kernel.org> wrote:
> > 
> > No rush, it's a clear fix anyway so we can queue this up
> > in the rc phase of the next kernel cycle!
> 

-- 
Andreas Klinger
Grabenreith 27
84508 Burgkirchen
+49 8623 919966
ak@it-klinger.de
www.it-klinger.de
www.grabenreith.de

Re: [PATCH] iio: srf04: fix wrong limitation in distance measuring

[Zbyněk Kocur]

[-- Attachment #1: Type: text/plain, Size: 2031 bytes --]

Hi Andreas,

I'm so sorry for the delay. I was working on another project now, and I wasn't entirely devoted to this. I looked at it at the weekend and the modified code is working. I have only tested OpenWRT in build - r10204-38b22b1, kernel 4.14.125. I do not expect complications on other platforms.

I tried it on several sensors. I was primarily interested in JSN-SR04T (https://www.makerguides.com/jsn-sr04t-arduino-tutorial/). I have also verified other designs (https://randomnerdtutorials.com/complete-guide-for-ultrasonic-sensor-hc-sr04/).

It generally works. Relatively accurate for the area up to 2.5 m. However, if the distance is longer, it starts to fizz, depending on the sensor design. It is a property of used HW and its construction.

I do not know how to do this in the development of drivers, whether it makes sense to put a filter directly into the driver or at least pull the maximum distance parameter as a variable?

Anyway, the change made works.

Zbynek
---
email: zbynek.kocur@fel.cvut.cz
phone: +420 224 354 054
web: http://www.fel.cvut.cz
Department of Telecommunications Engineering
Faculty of Electrical Engineering

> On 7 Sep 2019, at 11:46, Andreas Klinger <ak@it-klinger.de> wrote:
> 
> Hi Zbynek,
> 
> do you have any feedback for us?
> 
> Thank you,
> 
> Andreas
> 
> 
> Zbyněk Kocur <zbynek.kocur@fel.cvut.cz> schrieb am Mi, 26. Jun 21:50:
>> Thank you, i will write a feedback after testing.
>> 
>> 
>> Zbynek
>> ---
>> email: zbynek.kocur@fel.cvut.cz
>> phone: +420 224 354 054
>> web: http://www.fel.cvut.cz
>> Department of Telecommunications Engineering
>> Faculty of Electrical Engineering
>> 
>>> On 26 Jun 2019, at 21:49, Jonathan Cameron <jic23@kernel.org> wrote:
>>> 
>>> No rush, it's a clear fix anyway so we can queue this up
>>> in the rc phase of the next kernel cycle!
>> 
> 
> -- 
> Andreas Klinger
> Grabenreith 27
> 84508 Burgkirchen
> +49 8623 919966
> ak@it-klinger.de
> www.it-klinger.de
> www.grabenreith.de


[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 3620 bytes --]

Re: [PATCH] iio: srf04: fix wrong limitation in distance measuring

[Andreas Klinger]

Hi Zbyněk,

thanks a lot for testing and sharing your results with us.

> I do not know how to do this in the development of drivers, whether it makes
> sense to put a filter directly into the driver or at least pull the maximum
> distance parameter as a variable?

In this case one should read several times, remove spikes and calculate the
average. This sounds to me as task for the userspace not for the driver.

Regards,

Andreas


Zbyněk Kocur <zbynek.kocur@fel.cvut.cz> schrieb am Mo, 09. Sep 08:53:
> Hi Andreas,
> 
> I'm so sorry for the delay. I was working on another project now, and I wasn't entirely devoted to this. I looked at it at the weekend and the modified code is working. I have only tested OpenWRT in build - r10204-38b22b1, kernel 4.14.125. I do not expect complications on other platforms.
> 
> I tried it on several sensors. I was primarily interested in JSN-SR04T (https://www.makerguides.com/jsn-sr04t-arduino-tutorial/). I have also verified other designs (https://randomnerdtutorials.com/complete-guide-for-ultrasonic-sensor-hc-sr04/).
> 
> It generally works. Relatively accurate for the area up to 2.5 m. However, if the distance is longer, it starts to fizz, depending on the sensor design. It is a property of used HW and its construction.
> 
> I do not know how to do this in the development of drivers, whether it makes sense to put a filter directly into the driver or at least pull the maximum distance parameter as a variable?
> 
> Anyway, the change made works.
> 
> Zbynek
> ---
> email: zbynek.kocur@fel.cvut.cz
> phone: +420 224 354 054
> web: http://www.fel.cvut.cz
> Department of Telecommunications Engineering
> Faculty of Electrical Engineering
> 
> > On 7 Sep 2019, at 11:46, Andreas Klinger <ak@it-klinger.de> wrote:
> > 
> > Hi Zbynek,
> > 
> > do you have any feedback for us?
> > 
> > Thank you,
> > 
> > Andreas
> > 
> > 
> > Zbyněk Kocur <zbynek.kocur@fel.cvut.cz> schrieb am Mi, 26. Jun 21:50:
> >> Thank you, i will write a feedback after testing.
> >> 
> >> 
> >> Zbynek
> >> ---
> >> email: zbynek.kocur@fel.cvut.cz
> >> phone: +420 224 354 054
> >> web: http://www.fel.cvut.cz
> >> Department of Telecommunications Engineering
> >> Faculty of Electrical Engineering
> >> 
> >>> On 26 Jun 2019, at 21:49, Jonathan Cameron <jic23@kernel.org> wrote:
> >>> 
> >>> No rush, it's a clear fix anyway so we can queue this up
> >>> in the rc phase of the next kernel cycle!
> >> 
> > 
> > -- 
> > Andreas Klinger
> > Grabenreith 27
> > 84508 Burgkirchen
> > +49 8623 919966
> > ak@it-klinger.de
> > www.it-klinger.de
> > www.grabenreith.de
> 



-- 
Andreas Klinger
Grabenreith 27
84508 Burgkirchen
+49 8623 919966
ak@it-klinger.de
www.it-klinger.de
www.grabenreith.de