LacunaSpace / basicmac

BasicMAC LoRaWAN stack that supports (but is not limited to) Arduino
Other
76 stars 18 forks source link

u8_t/s8_t (re-)definition collisions #28

Closed Bwooce closed 3 years ago

Bwooce commented 3 years ago

So the esp32 folks like to think in bits, and basicmac likes to think in bytes, and so we end up with a conflicting definition. No one is right here, but having a conflict is annoying/dangerous. It starts to appear as a problem if you include (ESP32s) WiFi.h.

Would you accept a pull request to move to esp32-like thinking? (can postpone until after the merge you're planning, or raise this over there with @mkuyper ?)

Users/bruce/Library/Arduino15/packages/esp32/hardware/esp32/1.0.5-rc4/tools/sdk/include/lwip/lwip/arch.h:119:19: error: conflicting declaration 'typedef uint8_t u8_t'
 typedef uint8_t   u8_t;
                   ^
In file included from /Users/bruce/Documents/Arduino/libraries/BasicMAC/src/lmic/lmic.h:14:0,
                 from /Users/bruce/Documents/Arduino/libraries/BasicMAC/src/basicmac.h:33,
                 from /Users/bruce/Documents/Arduino/Lora-TTNMapper-T-Beam/Lora-TTNMapper-T-Beam.ino:1:
/Users/bruce/Documents/Arduino/libraries/BasicMAC/src/lmic/oslmic.h:30:25: note: previous declaration as 'typedef uint64_t u8_t'
 typedef uint64_t        u8_t;
matthijskooijman commented 3 years ago

Thanks for raising this. The same thing came up on LMIC, there I fixed this by just removing u8_t and s8_t (and using uint64_t and int64_t directly).

In general, I wouldn't want to fix this by replacing the byte-oriented typedefs with bit-oriented typedefs, I'd rather just switch to the standard uintx_t macros, which would prevent all conflicts in a more reliable way.

Would you accept a pull request to move to esp32-like thinking? (can postpone until after the merge you're planning, or raise this over there with @mkuyper ?)

Probably best to leave this until the merge with @mkuyper's version is settled indeed.

Bwooce commented 3 years ago

Ah it's from the original IBM code? Ha, what's old is new again :)

Using standard types sounds good, for the future then: sed -I .bak 's/s8_t/int64_t/g; s/u8_t/uint64_t/g; s/s4_t/int32_t/g; s/u4_t/uint32_t/g; s/s2_t/int16_t/g; s/u2_t/uint16_t/g; s/s1_t/int8_t/g; s/u1_t/uint8_t/g; s/bit_t/uint8_t/g;' ./lmic/*

and remove the typedef lines in oslmic.h

(I'll try and remember to do a pull request to github.com/mkuyper/basicmac once it's all settled. If that's the final master? )

matthijskooijman commented 3 years ago

Thanks!

(I'll try and remember to do a pull request to github.com/mkuyper/basicmac once it's all settled. If that's the final master? )

That's where we're headed, yes.

mkuyper commented 3 years ago

Yes, I'd definitely accept a PR for that. These types are a remnant of IBM's MoteRunner, the platform that the pre-cursor to LMiC was based on -- and C99 was too radical a requirement back then. There's no technical reason anymore to not use the stdint.h types directly now.