InfiniTimeOrg / InfiniTime

Firmware for Pinetime smartwatch written in C++ and based on FreeRTOS
GNU General Public License v3.0
2.64k stars 902 forks source link

p8: Fix build when building for P8 variants #1913

Closed FintasticMan closed 6 months ago

FintasticMan commented 7 months ago

Building with a TARGET_DEVICE set to any of the P8 variants' names caused the build to fail, because they contained hyphens. The build defines a macro TARGET_DEVICE_$VARIANT, which fails if $VARIANT contains a hyphen.

github-actions[bot] commented 7 months ago
Build size and comparison to main: Section Size Difference
text 377496B -16B
data 940B 0B
bss 63420B 0B
NeroBurner commented 7 months ago

@StarGate01 I think you did the initial support for p8 builds, could you have a look at this please

NeroBurner commented 7 months ago

It seems the TARGETDEVICE${VARIANT} definition is never used anywhere 🤷 only the TARGET_DEVICE_NAME=${TARGET_DEVICE} one is used

I don't know if the MOY-TFK5 et.al. strings have a meaning, and if the - is important in that name. The changed _ will show up in the info screen of the device.

So if we want to keep the - in the string we should be golden if we just remove the TARGET_DEVICE_${TARGET_DEVICE} define

FintasticMan commented 7 months ago

I did think of that, but I don't think it's very important to keep the strings exactly as they were tbh. It would be good to have @StarGate01 take a look at it. I think it is good to have the TARGET_DEVICE_${TARGET_DEVICE} macro, so that the code can check what device it is without having to use strcmp or something like that.

StarGate01 commented 6 months ago

Good catch, I actually fixed this in my fork a while back since I had the same issue: https://github.com/StarGate01/InfiniTime/commit/3e0841dd5d9b3fe08f85f1d468fcd968fd3281c0 .

I'll eventually rebase my fork and update the PRs (https://github.com/InfiniTimeOrg/InfiniTime/pulls/stargate01)

NeroBurner commented 6 months ago

Thanks for the feedback! Nice we found the same solution independently 😁