linguini1 commented on issue #15856: URL: https://github.com/apache/nuttx/issues/15856#issuecomment-2705122977
> @linguini1 > Merged the new things. I'm looking which added IOCTLs of you can stay or move. > > - WLIOC_SETBITRATE, is this for GFSK? Then lets move that under GFSK tag Yeah, it's for FSK modulation. > - WLIOC_CRCEN -> WLIOC_LORA_ENCRC > - WLIOC_SETCODERATE -> WLIOC_LORA_SETCR (with an enum) > - WLIOC_IQEN can stay. Good idea for common. > - WLIOC_SETSYNC -> WLIOC_LORA_SYNCWORD > - WLIOC_SETMOD can stay for common > - WLIOC_SETSPREAD -> WLIOC_LORA_SF > - WLIOC_SETBANDWIDTH -> WLIOC_LORA_BW Looks good to me! > - WLIOC_GETSNR -> in the rx_header, since its common packet specific, right? Yes, but I'm still thinking about that approach. I personally almost feel it's better to keep `read()` implemented such that it just stores the recieved packet in the user buffer. Anything else strays too far from the POSIX idea of read imo. For instance, a really great testing use case for me is to be able to swap the fd being passed to read. Instead of the radio, I can mock this using packet data sent over USB or UART or something like that. If I have to structure the read buffer, this removes that easy feature. I think users should separately query for the SNR. I find this a flaw with other NuttX drivers, like the ADC standard driver. If the read buffer has to be structured, I think a different interface is required. > WLIOC_SETTXPOWERF is the same as WLIOC_SETTXPOWER? I think so far this is the only breaking change. The former is for float arguments to keep backward compatibility with raiden's driver, but we can toss that in favour of the latter which uses integers of 0.01dBm steps. > I'm not entirely sure if we really need that fine precision afterall. Nearly all chips only do steps of 1dB. I recommend at least a step of 0.1dBm. The RN2XX3 at least has that much precision. Idk about other chips. > If we do really need it, we should probably make this fixedmath.h b16_t. I'm really against fixedmath because I find it unintuitive and it requires the user to use nonstandard representations for numbers. C standard numbers are my preference by far. > Do we really need to GET the values too? Everything is usually reset after closing the driver anyway. I think some of them yes. For instance, setting TX power may round to the next highest level the chip supports. It would be good for the user to be able to check. I guess we can store the real value back in the user pointer though. It may also be useful to check some things like modulation, etc. What if someone's application involves slowly increasing TX power over time (maybe distance is increasing). Having the GET commands let's them check TX power and increment it instead of maintaining their own state. Same thing could be said for other GETs. I'm not against just leaving some out for now until there's a use case for it, but whatever we standardize is going to be hard to add to after the fact for all drivers. > What do you think of that? > > _Also Your driver has a sneaky bug in it WLIOC_GETTXPOWERF is 0x0020. You accidently incremented from 0x19 to 0x20. Should be 0x1a lol._ > Oops! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
