mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Marcus Wolf <marcus.wolf@smarthome-wolf.de>
To: Hugo Lefeuvre <hle@owl.eu.com>
Cc: Valentin Vidic <Valentin.Vidic@CARNet.hr>,
	devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Subject: Re: rf69_set_deviation in rf69.c (pi433 driver)
Date: Tue, 19 Jun 2018 12:19:14 +0200	[thread overview]
Message-ID: <50c4e6ee-e5fb-1d27-4e73-1e82080036b3@smarthome-wolf.de> (raw)
In-Reply-To: <20180605031120.GB1981@hle-laptop.local>

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

  reply	other threads:[~2018-06-19 10:24 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-05  3:11 Hugo Lefeuvre
2018-06-19 10:19 ` Marcus Wolf [this message]
2018-06-22  2:03   ` Hugo Lefeuvre
2018-06-22  8:46     ` Valentin Vidic

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=50c4e6ee-e5fb-1d27-4e73-1e82080036b3@smarthome-wolf.de \
    --to=marcus.wolf@smarthome-wolf.de \
    --cc=Valentin.Vidic@CARNet.hr \
    --cc=devel@driverdev.osuosl.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=hle@owl.eu.com \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®