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 <[email protected]>
Signed-off-by: Andreas Klinger <[email protected]>
---
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
On Sun, 23 Jun 2019 14:29:10 +0200
Andreas Klinger <[email protected]> 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 <[email protected]>
> Signed-off-by: Andreas Klinger <[email protected]>
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;
> }
Thank you, i will write a feedback after testing.
Zbynek
---
email: [email protected]
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 <[email protected]> wrote:
>
> No rush, it's a clear fix anyway so we can queue this up
> in the rc phase of the next kernel cycle!
On Wed, 26 Jun 2019 20:59:08 +0200
Zbyněk Kocur <[email protected]> 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: [email protected]
> 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 <[email protected]> wrote:
> >
> > On Sun, 23 Jun 2019 14:29:10 +0200
> > Andreas Klinger <[email protected]> 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 <[email protected]>
> >> Signed-off-by: Andreas Klinger <[email protected]>
> > 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;
> >> }
> >
>
Hi Zbynek,
do you have any feedback for us?
Thank you,
Andreas
Zbyněk Kocur <[email protected]> schrieb am Mi, 26. Jun 21:50:
> Thank you, i will write a feedback after testing.
>
>
> Zbynek
> ---
> email: [email protected]
> 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 <[email protected]> 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
[email protected]
http://www.it-klinger.de
http://www.grabenreith.de
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: [email protected]
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 <[email protected]> wrote:
>
> Hi Zbynek,
>
> do you have any feedback for us?
>
> Thank you,
>
> Andreas
>
>
> Zbyněk Kocur <[email protected]> schrieb am Mi, 26. Jun 21:50:
>> Thank you, i will write a feedback after testing.
>>
>>
>> Zbynek
>> ---
>> email: [email protected]
>> 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 <[email protected]> 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
> [email protected]
> http://www.it-klinger.de
> http://www.grabenreith.de
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 <[email protected]> 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: [email protected]
> 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 <[email protected]> wrote:
> >
> > Hi Zbynek,
> >
> > do you have any feedback for us?
> >
> > Thank you,
> >
> > Andreas
> >
> >
> > Zbyněk Kocur <[email protected]> schrieb am Mi, 26. Jun 21:50:
> >> Thank you, i will write a feedback after testing.
> >>
> >>
> >> Zbynek
> >> ---
> >> email: [email protected]
> >> 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 <[email protected]> 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
> > [email protected]
> > http://www.it-klinger.de
> > http://www.grabenreith.de
>
--
Andreas Klinger
Grabenreith 27
84508 Burgkirchen
+49 8623 919966
[email protected]
http://www.it-klinger.de
http://www.grabenreith.de