search

BusPirate / Bus_Pirate

Community driven firmware and hardware for Bus Pirate version 3 and 4
625 stars 131 forks source link

Fix compile errors with XC16 v2.00 #165

Closed e28eta closed 1 year ago

e28eta commented 2 years ago

Fixes #163

I ran into the same issue compiling the firmware for the first time. These seem like pretty straightforward fixes to me, but I could be wrong. I have very little experience with the Bus Pirate, and was unable to even confirm that I'm hitting these code paths, but the firmware compiles now and I believe these are equivalent statements.

Compiler error:

../openocd.c:215:7: error: implicit declaration of function 'min'

Remove calls to min() since I can't find it in the headers. I suspect there is a min function we could use for readability, but I don't see it declared anywhere in XC16's microchip/xc16/v2.00/include directory, and there were only two instances in a single file, so this seemed okay.

Compiler error:

../proc_menu.c: In function 'remove_current_character_from_command_line': ../proc_menu.c:1847:50: error: comparison between pointer and integer

Instead of comparing an integer (aka char) against NULL, simply use the short form of the ternary operator. Documented here: https://gcc.gnu.org/onlinedocs/gcc/Conditionals.html

mleyden commented 2 years ago

I used fmin() from and replaced NULL with '\0'

mleyden commented 2 years ago

In addition, I had linker problems with XC16 on Linux. I had to modify /busPirate.X/nbproject/configurations.xml and remove references to the C30 lib pic30-elf: and . I added

/opt/microchip/xc16/v2.00/lib/libc99-pic30-elf.a
e28eta commented 2 years ago

I used fmin() from and replaced NULL with '\0'

Afaik those are reasonable choices too. I didn't want to use fmin since this is dealing with integers and I didn't check / don't know whether fmin would trigger an unnecessary conversion to float.

I think NULL, '\0 and 0 would all be equivalent (modulo NULL triggering this compiler warning). I personally chose the elvis operator ?: because it's something I'm familiar with and IMO is more readable when you are.

In addition, I had linker problems with XC16 on Linux. I had to modify /busPirate.X/nbproject/configurations.xml and remove references to the C30 lib pic30-elf: and . I added /opt/microchip/xc16/v2.00/lib/libc99-pic30-elf.a

I saw that too with the Bus Pirate v4 configuration. I managed a successful compile by simply deleting the lib reference, but without the hardware to test on, I didn't want to proffer a fix.

@mleyden I have no idea how active this project is, nor how discriminating the maintainers are, but sounds to me like you've got a set of changes that're a totally reasonable alternative PR to this one. If I were you I'd submit, and maybe we can learn together about which choices are better?

mleyden commented 2 years ago

Finally uploaded my changes - see #167