On Tue, 8 Apr 2025 at 14:49, Xiang Xiao <xiaoxiang781...@gmail.com> wrote:
> On Wed, Apr 9, 2025 at 2:02 AM Lwazi Dube <lwa...@gmail.com> wrote:
>
> > On Tue, 8 Apr 2025 at 13:32, Xiang Xiao <xiaoxiang781...@gmail.com>
> wrote:
> >
> > > On Wed, Apr 9, 2025 at 1:16 AM Lwazi Dube <lwa...@gmail.com> wrote:
> > >
> > > > On Tue, 8 Apr 2025 at 05:23, Sebastien Lorquet <sebast...@lorquet.fr
> >
> > > > wrote:
> > > >
> > > > > Hello,
> > > > >
> > > > > Nathan proposal to have a kconfig, with a default to the historical
> > > > > algorithm, is a good solution.
> > > > >
> > > >
> > > > Kconfig will work if you mean this:
> > > >
> > > > int16_t crc16(FAR const uint8_t *src, size_t len)
> > > > {
> > > > #if defined(CONFIG_CRC16_IBM)
> > > > return crc16ibmpart(src, len, 0);
> > > > #elif defined(CONFIG_CRC16_XMODEM)
> > > > return crc16xmodempart(src, len, 0);
> > > > #endif
> > > > }
> > > >
> > >
> > > It's a very bad idea to use Kconfig to change the function's behaviour
> > like
> > > this.
> > >
> > For example, y/zmodem calls crc16 which expects the result same as
> > > crc16xmodempart.
> > > With this change, y/zmodem works fine when you select
> > CONFIG_CRC16_XMODEM,
> > > but stops working when you select CONFIG_CRC16_IBM.
> > > To make y/zmodem working with any choice, you have to change crc16 to
> > > crc16xmodempart.
> > > All code which calls crc16 before, must change to either crc16ibmpart
> or
> > > crc16xmodempart, which means crc16 can't be used in any portable code.
> > > Do you really want this behaviour?
> > >
> > No. Going forward, you should call crc16 if you want IBM. Otherwise call
> > the right thing.
> >
> Kconfig was for preserving historical nuttx behaviour for those who don't
> > want to keep up with upstream. It is not perfect.
> >
>
> So, we should change onewire_crc16 to crc16 per your suggestion:
>
> https://github.com/apache/nuttx/blob/master/drivers/1wire/1wire_crc.c#L73-L98
> When a user upgraded to this version with CONFIG_CRC16_XMODEM, 1wire driver
> stopped working suddenly.
> Changing the function behaviour by Kconfig doesn't improve any
> compatibility, but makes the user write the portable code very hard.
>
Agreed.
Someone else suggested Kconfig. I agreed and typed the code to make sure I
understood what they meant.
I think you have given enough reasons for me to abandon the Kconfig idea.
And there is enough resistance in the replies :)
