[PATCH 2/4] convert internal frequencies to KHz
Jouni Malinen
j at w1.fi
Fri Feb 26 17:08:55 EST 2021
On Wed, Jun 17, 2020 at 11:15:32PM -0700, Thomas Pedersen wrote:
> In preparation for supporting (S1G) channels centered on
> non-integer MHz, such as 902.5Mhz, convert nearly all
> internal frequency representations into units of KHz.
This is a huge patch and very likely to cause conflicts with any other
pending work.. That's one of the reasons why I did not really want to
even consider this while there was quite a few pending patches in the
queue. That situation is now better, so I can at least consider. Still,
it would be nice if this patch could be split into smaller pieces that
would go through interfaces in steps rather than everything at once.
> A conversion must be done when:
>
> - parsing existing API, ie. control interface, dbus, etc.
> - parsing config files
> - sending driver commands
> - receiving driver events
That sounds reasonable.
> When the user input is a string, we parse the input as a
> MHz float, then convert to KHz. This will existing
> commands to work on S1G channels unchanged.
But I don't really want to see floating point types for frequencies.
Integers in kHz with helper functions for parsing might be OK, but
ideally, I would not see even a single float or atof() in this patch..
> When generating events and log messages, we exploit the
> "%g" format specifier which will drop the "." and
> trailing zeroes if the input does not have a fractional
> component. Similar to parsing user input this lets us
> expose fractional MHz frequencies without disturbing
> existing users.
Needing floating point numbers for printing out integer values looks
undesired.. I guess "freq=902.5" might be more convenient to see in many
places instead of something like freq_khz=902500, though.. Anyway,
generating that with "%u.%u" or "%u.%03u" for 902.500 might be simpler.
> While dbus, driver_bsd, driver_atheros, driver_hostap,
> driver_privsep, driver_wext, etc. have been converted
> based on 'git grep freq ...', they have received no
> testing :(. Suggestions, extra review, or help testing
> these are welcome.
This is one of the major issues here. Alternative approach would be to
introduce new frequency-in-kHz variables for the places that really need
this while leaving the existing -in-MHz variants available to avoid
regressions.
And on the editorial front, I'd prefer correct spelling of the SI
unit/prefix (kHz) instead of that Kelvin Herz thingie used here in most
places.. ;-)
> This commit should introduce no functional change.
Maybe so, but this is going to hugely inconvenient to review and show to
be correct.
--
Jouni Malinen PGP id EFC895FA
More information about the Hostap
mailing list