Hi all, Thank you for thinking along! I've noted all these down.
*KConfig warning*Good idea but I agree that adding Kconfig that does little to nothing might not be the best idea. It feels awkward to me knowing the kconfig is indeed already quite complicated. Next to that, the experimental marking is everywhere, in comments, ioctl and docs. *Change but no breaking*I agree and this change can be purely *additive* instead of a *replacement*. I personally feel awkward about breaking the IOCTL command to change power in dBm to 1/100 steps. It makes practically no sense to have decibels in 1/100 steps. Even 1/10 is usually for precision equipment. 1/1 steps are fine enough for nearly all applications. Maybe for this instance, a driver specific precision dBm power set IOCTL might be a solution. This removes the "breaking change" from the equation. (However, see below) *Legacy config* That is totally a solution as well. Which might even solve the problem above *"dBm 1/100 vs dBm 1/1".*Also might be able to disable the old commands to save some memory. Though, I have felt resistance from some people about legacy and adding kconfig. I'm happy to hear more. Op vr 14 mrt 2025 om 11:53 schreef Sebastien Lorquet <sebast...@lorquet.fr>: > Hello > > For me the wrong is not to change, but to break the old working thing. > > The old IOCTLs can be maintained. Either by design, or as a config > option like CONFIG_WLIOC_ENABLE_LEGACY with default yes. > > That would work for me, I think, as it would allow running old code > without changing it (or just by adding a build option). > > > After all, linux has several syscalls doing the same thing with various > options. Documentation says it's totally deprecated, but it still works. > > Sebastien > > > On 14/03/2025 11:10, Xiang Xiao wrote: > > It's totally wrong to change interface definition or meaning by Kconfig. > > Interfaces mean some contract between kernel and userspace, how do > > applications follow the changing contract? > > > > On Fri, Mar 14, 2025 at 7:47 AM Nathan Hartman <hartman.nat...@gmail.com > > > > wrote: > > > >> Hi all, > >> > >> I like the fact that there has been a high level analysis here and an > >> effort to make things consistent across multiple types of > communications. > >> > >> Since the drivers mentioned are experimental, I agree that breaking > changes > >> aren't breaking stabilized interfaces. > >> > >> We should just be careful to mention this in the release notes because > this > >> could create a long debugging session for someone. > >> > >> Here's an idea: there could be a Kconfig that must be defined if using > any > >> of these drivers. Some kind of > >> CONFIG_I_KNOW_ABOUT_THE_BREAKING_CHANGES_IN_THE_RF_DRIVERS. (That should > >> *not* be its name; I'm only writing it to illustrate my idea.) If this > >> Kconfig isn't present, #ifdef logic in the source code will deliberately > >> break the build with #error directive, and adjacent documentation in the > >> comments. This will ensure that nobody using the ioctl as it currently > is > >> will have any head scratching and long debugging session. Thoughts? > >> > >> Nathan > >> > >> On Thu, Mar 13, 2025 at 4:50 PM Kevin Witteveen <kevinwit1...@gmail.com > > > >> wrote: > >> > >>> Hi Nuttx, > >>> > >>> This is a follow-up to GitHub issue #15856 > >>> <https://github.com/apache/nuttx/issues/15856> initiated by > raiden00pl. > >>> The > >>> discussion there provides useful context for this proposal. > >>> > >>> *--- Summary ---* > >>> Currently, the IOCTL API for character-driven RF devices lacks a common > >>> interface across different modulation technologies, such as LoRa, FSK, > >> and > >>> OOK. As a result, driver-specific IOCTL commands are created even when > >> they > >>> could be shared across multiple radios. This fragmentation makes > >>> application portability more difficult to maintain. > >>> > >>> *--- Impact ---* > >>> This does require some changes to be made to 4 RF drivers. > >>> > >>> - My SX126x (New, experimental) > >>> - Raiden SX127x, > >>> - Linguini RN2xx3 (New, experimental) > >>> - nRF > >>> > >>> The three of us—Raiden, Linguini, and I—support this idea and are > already > >>> involved in the development. Once implemented, this common API would > make > >>> it easier for IoT applications to run on NuttX with minimal > >> driver-specific > >>> modifications, reducing maintenance overhead and eliminating redundant > >>> implementations. > >>> > >>> *--- Breaking ---* > >>> This is considered a breaking change if the following IOCTL command may > >> be > >>> changed: > >>> > >>> WLIOC_SETTXPOWER takes an integer value for dB. This can be changed to > >>> steps of 1/100 dB instead. Because the RN2xx3 and some radios can take > >>> smaller values. (*Open for discussion)* > >>> > >>> However, the RN2xx3 and SX126x drivers are marked experimental, so any > >>> modifications to these are *not* considered breaking. > >>> > >>> *--- Strategy ---* > >>> > >>> 1. Introduce new IOCTL commands without modifying legacy > >>> implementations. > >>> 2. Gradually migrate existing drivers to the new API while testing > and > >>> documenting. > >>> 3. Remove legacy implementations > >>> > >>> To avoid breaking changes, we could retain the legacy APIs if desired > and > >>> only remove the *experimental* ones. > >>> > >>> *--- Proposal API header ---* > >>> > >>> > >> > https://github.com/keever50/nuttx/blob/common_wireless_character_driver_API/include/nuttx/wireless/ioctl.h > >>> I collaborated with Raiden and Linguini on defining these IOCTL > commands. > >>> Feedback and suggestions from the community would be greatly > appreciated. > >>> > >>> *--- Note about "Experimental" ---* > >>> The SX126x and RN2xx3 drivers are marked as experimental, meaning users > >> are > >>> explicitly warned that changes may occur in the near future. They were > >>> labeled as such before being merged. > >>> > >>> Looking forward to your thoughts! > >>> > >>> Best regards, > >>> > >>> Kevin (Keever50 on github) > >>> >
