lancaster-university / codal-microbit-v2

CODAL target for the micro:bit v2.x series of devices
MIT License
41 stars 50 forks source link

MicroBitButton.h missing in MicroBit.h when DEVICE_BLE=0 #403

Closed tballmsft closed 4 months ago

tballmsft commented 4 months ago

When building pxt-microbit with DEVICE_BLE=0 against this repo, we get a compile error as MicroBitButton is not defined. I traced this down to the following issue in MicroBit.h in this repo:

MicroBitButton.h is not included directly in MicroBit.h, but only indirectly by MicroBitButtonService.h, which is only included in MicroBit.h when DEVICE_BLE=1

finneyj commented 4 months ago

Thanks @tballmsft!

Easy one - can we squeeze a fix for this into our planned next release @JohnVidler?

martinwork commented 4 months ago

I suspect MicroBitButton.h was only included in the BLE code because it was in the V1 code, and it's really a mistake: MicroBitButtonService doesn't seem to use MicroBitButton and should be using DEVICE macros rather than MICROBIT.

MicroBitButton is in the compat folder and only occurs here https://github.com/microsoft/pxt-microbit/blob/2dccb52fcfe9edd6a420cfd5718c4e80452a03ad/libs/core/pins.cpp#L647

tballmsft commented 4 months ago

It's not critical to fix this in lancaster, as we have a solution for pxt-microbit. In libs\core\pxtcore.h, I also see

#ifndef __PXTCORE_H
#define __PXTCORE_H

#include "MicroBit.h"
#include "MicroBitImage.h"
#include "ManagedString.h"
#include "ManagedType.h"

So clearly MicroBit.h wasn't designed to include everything we need in pxt-microbit, across V1 and V2, but to provide compat with V1.

tballmsft commented 4 months ago

Nevertheless, it's odd to have MicroBitButton.h appear in MicroBit.h for one build and not for another.

microbit-carlos commented 4 months ago

Okay, so it looks like we just need to include MicroBitButton.h into MicroBitCompat.h, as MicroBitCompat.h is already included in MicroBit.h.

microbit-carlos commented 4 months ago

Fixed in v0.2.65 by including MicroBitButton.h in MicroBitCompat.h: https://github.com/lancaster-university/codal-microbit-v2/commit/403ba81436236f2d5fd80db0c390f4f97bdb5882 https://github.com/lancaster-university/codal-microbit-v2/commit/0c67e627f29eaaef1c7821710eebc75b73cd0441