Re: rf69_set_deviation in rf69.c (pi433 driver)
On Thu, Jun 21, 2018 at 10:03:45PM -0400, Hugo Lefeuvre wrote: > I'll prepare a patch addressing both issues. However I don't own test devices > so it would be really great if you could test it ! I have two pi433 devices now so I should be able to tests things, just let me know... -- Valentin
Re: rf69_set_deviation in rf69.c (pi433 driver)
On Thu, Jun 21, 2018 at 10:03:45PM -0400, Hugo Lefeuvre wrote: > I'll prepare a patch addressing both issues. However I don't own test devices > so it would be really great if you could test it ! I have two pi433 devices now so I should be able to tests things, just let me know... -- Valentin
Re: rf69_set_deviation in rf69.c (pi433 driver)
Hi Marcus, > > According to the datasheet[0], the deviation should always be smaller > > than 300kHz, and the following equation should be respected: > > > > (1) FDA + BRF/2 =< 500 kHz > > > > Why did you choose 500kHz as max for FDA, instead of 300kHz ? It looks like > > a bug to me. > > My focus was always on OOK and ASK. PSK was only used for a few > measurements in the laboratory, I engaged to check CE compliance. > Most probably 500kHz was a value, that's common for PSK and I didn't pay > any attention to the datasheet. So I think, you are right: This is a bug > and could be revised. > Never the less: While using it in the lab, the transmission was fine and > the signals over air have been clean and fitted to the recommondations > of the CE. > > > > Concerning the TODO, I can see two solutions currently: > > > > 1. Introduce a new rf69_get_bit_rate function which reads REG_BITRATE_MSB > >and REG_BITRATE_LSB and returns reconstructed BRF. > >We could use this function in rf69_set_deviation to retrieve the BRF. > > > > + clean > > + intuitive > > - heavy / slow > > Why not: I like clean and intuitive implementations. Since it's used > during configuration, we shouldn't be that squeezed in time, that we > need to hurry. > > > > 2. Store BRF somewhere in rf69.c, initialize it with the default value > >(4.8 kb/s) and update it when rf69_set_bit_rate is called. > > > > + easy > > - dirty, doesn't fit well with the design of rf69.c (do not store > > anything, simply expose API) > > Up to my experience, storing reg values is always a bit problematic, > since the value may be outdated. And if you update the value every time > you want to use it to be sure, it's correct, there is no win in having > the copy of the reg value. > So this wouldn't be my favourite. > > > > I'd really prefer going for the first one, but I wanted to have your opinion > > on this. > > Agree. I'll prepare a patch addressing both issues. However I don't own test devices so it would be really great if you could test it ! I'm currently thinking of adapting this driver for other HopeRf modules like RFM69HCW or RFM12 so I will probably try to find some test equipement in the future. Thanks for your answer ! Regards, Hugo -- Hugo Lefeuvre (hle)|www.owl.eu.com 4096/ 9C4F C8BF A4B0 8FC5 48EB 56B8 1962 765B B9A8 BACA signature.asc Description: PGP signature
Re: rf69_set_deviation in rf69.c (pi433 driver)
Hi Marcus, > > According to the datasheet[0], the deviation should always be smaller > > than 300kHz, and the following equation should be respected: > > > > (1) FDA + BRF/2 =< 500 kHz > > > > Why did you choose 500kHz as max for FDA, instead of 300kHz ? It looks like > > a bug to me. > > My focus was always on OOK and ASK. PSK was only used for a few > measurements in the laboratory, I engaged to check CE compliance. > Most probably 500kHz was a value, that's common for PSK and I didn't pay > any attention to the datasheet. So I think, you are right: This is a bug > and could be revised. > Never the less: While using it in the lab, the transmission was fine and > the signals over air have been clean and fitted to the recommondations > of the CE. > > > > Concerning the TODO, I can see two solutions currently: > > > > 1. Introduce a new rf69_get_bit_rate function which reads REG_BITRATE_MSB > >and REG_BITRATE_LSB and returns reconstructed BRF. > >We could use this function in rf69_set_deviation to retrieve the BRF. > > > > + clean > > + intuitive > > - heavy / slow > > Why not: I like clean and intuitive implementations. Since it's used > during configuration, we shouldn't be that squeezed in time, that we > need to hurry. > > > > 2. Store BRF somewhere in rf69.c, initialize it with the default value > >(4.8 kb/s) and update it when rf69_set_bit_rate is called. > > > > + easy > > - dirty, doesn't fit well with the design of rf69.c (do not store > > anything, simply expose API) > > Up to my experience, storing reg values is always a bit problematic, > since the value may be outdated. And if you update the value every time > you want to use it to be sure, it's correct, there is no win in having > the copy of the reg value. > So this wouldn't be my favourite. > > > > I'd really prefer going for the first one, but I wanted to have your opinion > > on this. > > Agree. I'll prepare a patch addressing both issues. However I don't own test devices so it would be really great if you could test it ! I'm currently thinking of adapting this driver for other HopeRf modules like RFM69HCW or RFM12 so I will probably try to find some test equipement in the future. Thanks for your answer ! Regards, Hugo -- Hugo Lefeuvre (hle)|www.owl.eu.com 4096/ 9C4F C8BF A4B0 8FC5 48EB 56B8 1962 765B B9A8 BACA signature.asc Description: PGP signature
Re: rf69_set_deviation in rf69.c (pi433 driver)
Hi Hugo, sorry for the late response and thank you for all that deep investigation in the pi433 driver! > According to the datasheet[0], the deviation should always be smaller > than 300kHz, and the following equation should be respected: > > (1) FDA + BRF/2 =< 500 kHz > > Why did you choose 500kHz as max for FDA, instead of 300kHz ? It looks like > a bug to me. My focus was always on OOK and ASK. PSK was only used for a few measurements in the laboratory, I engaged to check CE compliance. Most probably 500kHz was a value, that's common for PSK and I didn't pay any attention to the datasheet. So I think, you are right: This is a bug and could be revised. Never the less: While using it in the lab, the transmission was fine and the signals over air have been clean and fitted to the recommondations of the CE. > > Concerning the TODO, I can see two solutions currently: > > 1. Introduce a new rf69_get_bit_rate function which reads REG_BITRATE_MSB >and REG_BITRATE_LSB and returns reconstructed BRF. >We could use this function in rf69_set_deviation to retrieve the BRF. > > + clean > + intuitive > - heavy / slow Why not: I like clean and intuitive implementations. Since it's used during configuration, we shouldn't be that squeezed in time, that we need to hurry. > > 2. Store BRF somewhere in rf69.c, initialize it with the default value >(4.8 kb/s) and update it when rf69_set_bit_rate is called. > > + easy > - dirty, doesn't fit well with the design of rf69.c (do not store > anything, simply expose API) Up to my experience, storing reg values is always a bit problematic, since the value may be outdated. And if you update the value every time you want to use it to be sure, it's correct, there is no win in having the copy of the reg value. So this wouldn't be my favourite. > > I'd really prefer going for the first one, but I wanted to have your opinion > on this. Agree. > Thanks for your work ! Your welcome :-) Marcus
Re: rf69_set_deviation in rf69.c (pi433 driver)
Hi Hugo, sorry for the late response and thank you for all that deep investigation in the pi433 driver! > According to the datasheet[0], the deviation should always be smaller > than 300kHz, and the following equation should be respected: > > (1) FDA + BRF/2 =< 500 kHz > > Why did you choose 500kHz as max for FDA, instead of 300kHz ? It looks like > a bug to me. My focus was always on OOK and ASK. PSK was only used for a few measurements in the laboratory, I engaged to check CE compliance. Most probably 500kHz was a value, that's common for PSK and I didn't pay any attention to the datasheet. So I think, you are right: This is a bug and could be revised. Never the less: While using it in the lab, the transmission was fine and the signals over air have been clean and fitted to the recommondations of the CE. > > Concerning the TODO, I can see two solutions currently: > > 1. Introduce a new rf69_get_bit_rate function which reads REG_BITRATE_MSB >and REG_BITRATE_LSB and returns reconstructed BRF. >We could use this function in rf69_set_deviation to retrieve the BRF. > > + clean > + intuitive > - heavy / slow Why not: I like clean and intuitive implementations. Since it's used during configuration, we shouldn't be that squeezed in time, that we need to hurry. > > 2. Store BRF somewhere in rf69.c, initialize it with the default value >(4.8 kb/s) and update it when rf69_set_bit_rate is called. > > + easy > - dirty, doesn't fit well with the design of rf69.c (do not store > anything, simply expose API) Up to my experience, storing reg values is always a bit problematic, since the value may be outdated. And if you update the value every time you want to use it to be sure, it's correct, there is no win in having the copy of the reg value. So this wouldn't be my favourite. > > I'd really prefer going for the first one, but I wanted to have your opinion > on this. Agree. > Thanks for your work ! Your welcome :-) Marcus
rf69_set_deviation in rf69.c (pi433 driver)
Hi Marcus,
I have been taking a look at the pi433 driver these last days, and started
working on the remaining TODOs. I just stumbled across the following
one (drivers/staging/pi433/rf69.c):
245 // TODO: Dependency to bitrate
246 if (deviation < 600 || deviation > 50) {
247 dev_dbg(>dev, "set_deviation: illegal input param");
248 return -EINVAL;
249 }
According to the datasheet[0], the deviation should always be smaller
than 300kHz, and the following equation should be respected:
(1) FDA + BRF/2 =< 500 kHz
Why did you choose 500kHz as max for FDA, instead of 300kHz ? It looks like
a bug to me.
Concerning the TODO, I can see two solutions currently:
1. Introduce a new rf69_get_bit_rate function which reads REG_BITRATE_MSB
and REG_BITRATE_LSB and returns reconstructed BRF.
We could use this function in rf69_set_deviation to retrieve the BRF.
+ clean
+ intuitive
- heavy / slow
2. Store BRF somewhere in rf69.c, initialize it with the default value
(4.8 kb/s) and update it when rf69_set_bit_rate is called.
+ easy
- dirty, doesn't fit well with the design of rf69.c (do not store
anything, simply expose API)
I'd really prefer going for the first one, but I wanted to have your opinion
on this.
Thanks for your work !
Best regards,
Hugo
[0] http://www.hoperf.com/upload/rf/RFM69CW-V1.1.pdf
[CC-ing Valentin Vidic, he was quite active on the pi433 driver these
last months]
--
Hugo Lefeuvre (hle)|www.owl.eu.com
4096/ 9C4F C8BF A4B0 8FC5 48EB 56B8 1962 765B B9A8 BACA
signature.asc
Description: PGP signature
rf69_set_deviation in rf69.c (pi433 driver)
Hi Marcus,
I have been taking a look at the pi433 driver these last days, and started
working on the remaining TODOs. I just stumbled across the following
one (drivers/staging/pi433/rf69.c):
245 // TODO: Dependency to bitrate
246 if (deviation < 600 || deviation > 50) {
247 dev_dbg(>dev, "set_deviation: illegal input param");
248 return -EINVAL;
249 }
According to the datasheet[0], the deviation should always be smaller
than 300kHz, and the following equation should be respected:
(1) FDA + BRF/2 =< 500 kHz
Why did you choose 500kHz as max for FDA, instead of 300kHz ? It looks like
a bug to me.
Concerning the TODO, I can see two solutions currently:
1. Introduce a new rf69_get_bit_rate function which reads REG_BITRATE_MSB
and REG_BITRATE_LSB and returns reconstructed BRF.
We could use this function in rf69_set_deviation to retrieve the BRF.
+ clean
+ intuitive
- heavy / slow
2. Store BRF somewhere in rf69.c, initialize it with the default value
(4.8 kb/s) and update it when rf69_set_bit_rate is called.
+ easy
- dirty, doesn't fit well with the design of rf69.c (do not store
anything, simply expose API)
I'd really prefer going for the first one, but I wanted to have your opinion
on this.
Thanks for your work !
Best regards,
Hugo
[0] http://www.hoperf.com/upload/rf/RFM69CW-V1.1.pdf
[CC-ing Valentin Vidic, he was quite active on the pi433 driver these
last months]
--
Hugo Lefeuvre (hle)|www.owl.eu.com
4096/ 9C4F C8BF A4B0 8FC5 48EB 56B8 1962 765B B9A8 BACA
signature.asc
Description: PGP signature
