Closed Brycey92 closed 4 weeks ago
You know, we also have OS detection in the makefile and we could use it to append something consistent to DEFINES_LIST
for each OS rather than check a dozen things a dozen places. What we have:
################################################################################
# What OS we're compiling on
################################################################################
ifeq ($(OS),Windows_NT)
HOST_OS = Windows
else
UNAME_S := $(shell uname -s)
ifeq ($(UNAME_S),Linux)
HOST_OS = Linux
else ifeq ($(UNAME_S),Darwin)
HOST_OS = Darwin
endif
endif
What we could add:
ifeq ($(HOST_OS),Windows)
DEFINES_LIST += WINDOWS=1
else ifeq ($(HOST_OS),Linux)
DEFINES_LIST += __linux__=1
else ifeq ($(UNAME_S),Darwin)
DEFINES_LIST += __APPLE__=1
endif
Admittedly this would only be for the emulator, not the tools folder.
Description
I've replaced a ton of preprocessor OS detections in rawdraw, cnfa, and the swadge repo. However, I didn't replace all of them, especially where it looked like the author was intentionally not using all possible environment variable names (that they knew of) to detect a certain OS.
This fixes an issue where
make usbflash
fails to work on Windows with the current toolchain.Respective PRs for the submodules can be found here for rawdraw and cnfa.
Test Instructions
As a warning, I've only tested this in a Windows Cygwin environment, so other environments and OSes still need to be tested.
Readiness Checklist
make format
failed with the following error:As for compile warnings, too many have slipped past into this repo for me to be able to easily check for new ones. I can check by hand, if required.
I have runmake format
to format the changesI have compiled the firmware and the changes have no warningsI have compiled the emulator and the changes have no warningsmake cppcheck
and checked thatcppcheck_result.txt
has no warnings for the changes/*! \file
comments with Design Philosophy, Usage, and Example sections for new headers.make docs
and checked thatdoxy_warnings.txt
has no warnings for the new code