Hi Will,

On 23 January 2017 at 18:43, will sanfilippo <[email protected]> wrote:
>
> Szymon:
>
> Indeed, those endianness macros were put in ble.h because they were 
> non-standard and acted on a buffer as opposed to just swapping bytes. 
> Internally (quite some time ago) we debated using packed structures for  PDU 
> protocol elements and we just never ended up deciding on what to do 
> throughout the code. We did figure if we went the packed structure route the 
> macros used (htole16) would get replaced with ones that just byte swap (if 
> needed).

Yeap, I'll work on that (starting with SM code).

>
> I looked over the changes and they look good to me. With these changes we 
> should also go through the code and use packed structures elsewhere. This 
> will definitely save a bunch of code as there will be no swapping since the 
> protocol and host are little endian.

OK, I'm sending pull request.

> I think there are also macros in the host for endianness-related functions. 
> Not sure if they have been renamed/replaced.

>From what I see host and controller are using same endianness helpers.
I'll convert any missing code if stumble upon.

>
> > On Jan 23, 2017, at 8:34 AM, Szymon Janc <[email protected]> wrote:
> >
> > Hi,
> >
> > While lurking in code I noticed that endianness APIs in Mynewt
> > are bit strange and scattered around:
> > - htole16, htobe16 etc are defined in "nimble/ble.h"
> > - above mentioned functions have signatures different than same named
> >  functions normally defined in endian.h
> >
> > So to clean those up I propose following:
> > - rename functions existing in ble.h to put_le16, get_le16 etc which are
> >   intended for use on raw byte buffer
> > - move those to endian.h
> > - add standard htole16 etc definitions in endian.h
> >
> > Some open points:
> > 1) there are two functions in ble.h
> >     void swap_in_place(void *buf, int len);
> >     void swap_buf(uint8_t *dst, const uint8_t *src, int len);
> >   that I also moved to endian.h for time being but I think that eventually
> >   we should have "os/misc.h" (or utils.h) for such helpers
> >
> > 2) I had to wrap macros in endian.h into #ifndef-endif since tests seem
> >   to be including both os/ and system includes resulting in macro redefined
> >   error
> >
> > Code implementing above is available at [1].
> >
> > Comments are welcome.
> >
> >
> > [1] https://github.com/sjanc/incubator-mynewt-core/commits/endianness
> >
> > --
> > pozdrawiam
> > Szymon K. Janc
>



-- 
pozdrawiam
Szymon K. Janc

Reply via email to