Skip to content

Fix multiple Simulator issues on Windows #22516

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
thinkyhead merged 12 commits into from
Aug 9, 2021
Merged

Fix multiple Simulator issues on Windows #22516

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
ad3801a
Add missing lib to native.ini
sjasonsmith Aug 5, 2021
60a9878
Inlcud u8glib-HAL in simulator
sjasonsmith Aug 5, 2021
c4ae886
Avoid duplicate function definitions in pinsDebug.h for simulator
sjasonsmith Aug 5, 2021
3f49469
Add default include path to build parameters for Windows simulator
sjasonsmith Aug 5, 2021
06e1faa
Remove integer type assumptions from serial print helpers.
sjasonsmith Aug 5, 2021
8a4524c
Define print for long long
sjasonsmith Aug 5, 2021
ad3ee53
Fix typos
sjasonsmith Aug 5, 2021
5b21247
Remove templates from printNumber_signed and printNumber_unsigned.
sjasonsmith Aug 5, 2021
ef2fbb1
Cosmetic alignment
sjasonsmith Aug 5, 2021
490377c
Fix ambiguous overloads related to PrintBase.
sjasonsmith Aug 5, 2021
22034f5
Remove unnecessary NO_INLINE and FORCE_INLINE attributes in serial_ba…
sjasonsmith Aug 6, 2021
68d1615
FORCE_INLINE print functions which simply pass through to another fun…
sjasonsmith Aug 6, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions Marlin/src/HAL/NATIVE_SIM/pinsDebug.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
* Support routines for X86_64
*/

#pragma once

/**
* Translation of routines & variables used by pinsDebug.h
*/
Expand All @@ -37,23 +39,23 @@
#define MULTI_NAME_PAD 16 // space needed to be pretty if not first name assigned to a pin

// active ADC function/mode/code values for PINSEL registers
constexpr int8_t ADC_pin_mode(pin_t pin) {
inline constexpr int8_t ADC_pin_mode(pin_t pin) {
return (-1);
}

int8_t get_pin_mode(pin_t pin) {
inline int8_t get_pin_mode(pin_t pin) {
if (!VALID_PIN(pin)) return -1;
return 0;
}

bool GET_PINMODE(pin_t pin) {
inline bool GET_PINMODE(pin_t pin) {
int8_t pin_mode = get_pin_mode(pin);
if (pin_mode == -1 || pin_mode == ADC_pin_mode(pin)) // found an invalid pin or active analog pin
return false;

return (Gpio::getMode(pin) != 0); //input/output state
}

bool GET_ARRAY_IS_DIGITAL(pin_t pin) {
inline bool GET_ARRAY_IS_DIGITAL(pin_t pin) {
return (!IS_ANALOG(pin) || get_pin_mode(pin) != ADC_pin_mode(pin));
}
2 changes: 1 addition & 1 deletion Marlin/src/HAL/NATIVE_SIM/u8g/u8g_com_st7920_sw_spi.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@

#if ENABLED(U8GLIB_ST7920)

#include <U8glib.h>
#include <U8glib-HAL.h>
#include "../../shared/Delay.h"

#undef SPI_SPEED
Expand Down
4 changes: 2 additions & 2 deletions Marlin/src/HAL/NATIVE_SIM/u8g/u8g_com_sw_spi.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@
#define SPI_SPEED 2 // About 2 MHz

#include <Arduino.h>
#include <U8glib.h>
#include <U8glib-HAL.h>

#ifdef __cplusplus
extern "C" {
Expand Down Expand Up @@ -209,7 +209,7 @@ uint8_t u8g_com_sw_spi_fn(u8g_t *u8g, uint8_t msg, uint8_t arg_val, void *arg_pt
#endif

#elif !ANY(TFT_COLOR_UI, TFT_CLASSIC_UI, TFT_LVGL_UI, HAS_MARLINUI_HD44780) && HAS_MARLINUI_U8GLIB
#include <U8glib.h>
#include <U8glib-HAL.h>
uint8_t u8g_com_sw_spi_fn(u8g_t *u8g, uint8_t msg, uint8_t arg_val, void *arg_ptr) {return 0;}
#endif // HAS_MARLINUI_U8GLIB && !U8GLIB_ST7920
#endif // __PLAT_NATIVE_SIM__
4 changes: 2 additions & 2 deletions Marlin/src/HAL/shared/cpu_exception/exception_arm.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ void hook_cpu_exceptions() {

unsigned long *vecAddr = (unsigned long*)get_vtor();
SERIAL_ECHOPGM("Vector table addr: ");
SERIAL_PRINTLN(get_vtor(), HEX);
SERIAL_PRINTLN(get_vtor(), PrintBase::Hex);

#ifdef VECTOR_TABLE_SIZE
uint32_t vec_size = VECTOR_TABLE_SIZE;
Expand All @@ -349,7 +349,7 @@ void hook_cpu_exceptions() {
alignas(128) static unsigned long vectable[VECTOR_TABLE_SENTINEL];

SERIAL_ECHOPGM("Detected vector table size: ");
SERIAL_PRINTLN(vec_size, HEX);
SERIAL_PRINTLN(vec_size, PrintBase::Hex);
#endif

uint32_t defaultFaultHandler = vecAddr[(unsigned)7];
Expand Down
11 changes: 0 additions & 11 deletions Marlin/src/core/serial.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,17 +76,6 @@ void serialprintPGM(PGM_P str) {
void serial_echo_start() { static PGMSTR(echomagic, "echo:"); serialprintPGM(echomagic); }
void serial_error_start() { static PGMSTR(errormagic, "Error:"); serialprintPGM(errormagic); }

void serial_echopair_PGM(PGM_P const s_P, serial_char_t v) { serialprintPGM(s_P); SERIAL_CHAR(v.c); }
void serial_echopair_PGM(PGM_P const s_P, const char *v) { serialprintPGM(s_P); SERIAL_ECHO(v); }
void serial_echopair_PGM(PGM_P const s_P, char v) { serialprintPGM(s_P); SERIAL_ECHO(v); }
void serial_echopair_PGM(PGM_P const s_P, int v) { serialprintPGM(s_P); SERIAL_ECHO(v); }
void serial_echopair_PGM(PGM_P const s_P, long v) { serialprintPGM(s_P); SERIAL_ECHO(v); }
void serial_echopair_PGM(PGM_P const s_P, float v) { serialprintPGM(s_P); SERIAL_DECIMAL(v); }
void serial_echopair_PGM(PGM_P const s_P, double v) { serialprintPGM(s_P); SERIAL_DECIMAL(v); }
void serial_echopair_PGM(PGM_P const s_P, unsigned char v) { serialprintPGM(s_P); SERIAL_ECHO(v); }
void serial_echopair_PGM(PGM_P const s_P, unsigned int v) { serialprintPGM(s_P); SERIAL_ECHO(v); }
void serial_echopair_PGM(PGM_P const s_P, unsigned long v) { serialprintPGM(s_P); SERIAL_ECHO(v); }

void serial_spaces(uint8_t count) { count *= (PROPORTIONAL_FONT_RATIO); while (count--) SERIAL_CHAR(' '); }

void serial_ternary(const bool onoff, PGM_P const pre, PGM_P const on, PGM_P const off, PGM_P const post/*=nullptr*/) {
Expand Down
27 changes: 12 additions & 15 deletions Marlin/src/core/serial.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,8 +177,8 @@ void SERIAL_ECHOLN(T x) { SERIAL_IMPL.println(x); }
template <typename T, typename U>
void SERIAL_PRINT(T x, U y) { SERIAL_IMPL.print(x, y); }

template <typename T, typename U>
void SERIAL_PRINTLN(T x, U y) { SERIAL_IMPL.println(x, y); }
template <typename T>
void SERIAL_PRINTLN(T x, PrintBase y) { SERIAL_IMPL.println(x, y); }

// Flush the serial port
inline void SERIAL_FLUSH() { SERIAL_IMPL.flush(); }
Expand Down Expand Up @@ -293,21 +293,18 @@ void serialprintPGM(PGM_P str);
//
// Functions for serial printing from PROGMEM. (Saves loads of SRAM.)
//
void serial_echopair_PGM(PGM_P const s_P, serial_char_t v);
void serial_echopair_PGM(PGM_P const s_P, const char *v);
void serial_echopair_PGM(PGM_P const s_P, char v);
void serial_echopair_PGM(PGM_P const s_P, int v);
void serial_echopair_PGM(PGM_P const s_P, long v);
void serial_echopair_PGM(PGM_P const s_P, float v);
void serial_echopair_PGM(PGM_P const s_P, double v);
void serial_echopair_PGM(PGM_P const s_P, unsigned char v);
void serial_echopair_PGM(PGM_P const s_P, unsigned int v);
void serial_echopair_PGM(PGM_P const s_P, unsigned long v);
inline void serial_echopair_PGM(PGM_P const s_P, serial_char_t v) { serialprintPGM(s_P); SERIAL_CHAR(v.c); }

inline void serial_echopair_PGM(PGM_P const s_P, float v) { serialprintPGM(s_P); SERIAL_DECIMAL(v); }
inline void serial_echopair_PGM(PGM_P const s_P, double v) { serialprintPGM(s_P); SERIAL_DECIMAL(v); }
inline void serial_echopair_PGM(PGM_P const s_P, const char *v) { serialprintPGM(s_P); SERIAL_ECHO(v); }

// Default implementation for types without a specialization. Handles integers.
template <typename T>
void serial_echopair_PGM(PGM_P const s_P, T v) { serialprintPGM(s_P); SERIAL_ECHO(v); }

inline void serial_echopair_PGM(PGM_P const s_P, bool v) { serial_echopair_PGM(s_P, (int)v); }
inline void serial_echopair_PGM(PGM_P const s_P, void *v) { serial_echopair_PGM(s_P, (uintptr_t)v); }
#if __INTPTR_WIDTH__ != __SIZE_WIDTH__
inline void serial_echopair_PGM(PGM_P const s_P, size_t v) { serial_echopair_PGM(s_P, (long int)v); }
#endif

void serial_echo_start();
void serial_error_start();
Expand Down
114 changes: 67 additions & 47 deletions Marlin/src/core/serial_base.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,12 +74,12 @@ CALL_IF_EXISTS_IMPL(SerialFeature, features, SerialFeature::None);
// for any type other than double/float. For double/float, a conversion exists so the call will be invisible.
struct EnsureDouble {
double a;
FORCE_INLINE operator double() { return a; }
operator double() { return a; }
// If the compiler breaks on ambiguity here, it's likely because print(X, base) is called with X not a double/float, and
// a base that's not a PrintBase value. This code is made to detect the error. You MUST set a base explicitly like this:
// SERIAL_PRINT(v, PrintBase::Hex)
FORCE_INLINE EnsureDouble(double a) : a(a) {}
FORCE_INLINE EnsureDouble(float a) : a(a) {}
EnsureDouble(double a) : a(a) {}
EnsureDouble(float a) : a(a) {}
};

// Using Curiously-Recurring Template Pattern here to avoid virtual table cost when compiling.
Expand Down Expand Up @@ -136,70 +136,90 @@ struct SerialBase {
void flushTX() { CALL_IF_EXISTS(void, SerialChild, flushTX); }

// Glue code here
FORCE_INLINE void write(const char *str) { while (*str) write(*str++); }
FORCE_INLINE void write(const uint8_t *buffer, size_t size) { while (size--) write(*buffer++); }
FORCE_INLINE void print(const char *str) { write(str); }
void write(const char *str) { while (*str) write(*str++); }
void write(const uint8_t *buffer, size_t size) { while (size--) write(*buffer++); }
void print(char *str) { write(str); }
void print(const char *str) { write(str); }
// No default argument to avoid ambiguity
NO_INLINE void print(char c, PrintBase base) { printNumber((signed long)c, (uint8_t)base); }
NO_INLINE void print(unsigned char c, PrintBase base) { printNumber((unsigned long)c, (uint8_t)base); }
NO_INLINE void print(int c, PrintBase base) { printNumber((signed long)c, (uint8_t)base); }
NO_INLINE void print(unsigned int c, PrintBase base) { printNumber((unsigned long)c, (uint8_t)base); }
void print(unsigned long c, PrintBase base) { printNumber((unsigned long)c, (uint8_t)base); }
void print(long c, PrintBase base) { printNumber((signed long)c, (uint8_t)base); }
void print(EnsureDouble c, int digits) { printFloat(c, digits); }

// Define print for every fundamental integer type, to ensure that all redirect properly
// to the correct underlying implementation.

// Prints are performed with a single size, to avoid needing multiple print functions.
// The fixed integer size used for prints will be the larger of long or a pointer.
#if __LONG_WIDTH__ >= __INTPTR_WIDTH__
typedef long int_fixed_print_t;
typedef unsigned long uint_fixed_print_t;
#else
typedef intptr_t int_fixed_print_t;
typedef uintptr_t uint_fixed_print_t;

FORCE_INLINE void print(intptr_t c, PrintBase base) { printNumber_signed(c, base); }
FORCE_INLINE void print(uintptr_t c, PrintBase base) { printNumber_unsigned(c, base); }
#endif

FORCE_INLINE void print(char c, PrintBase base) { printNumber_signed(c, base); }
FORCE_INLINE void print(short c, PrintBase base) { printNumber_signed(c, base); }
FORCE_INLINE void print(int c, PrintBase base) { printNumber_signed(c, base); }
FORCE_INLINE void print(long c, PrintBase base) { printNumber_signed(c, base); }
FORCE_INLINE void print(unsigned char c, PrintBase base) { printNumber_unsigned(c, base); }
FORCE_INLINE void print(unsigned short c, PrintBase base) { printNumber_unsigned(c, base); }
FORCE_INLINE void print(unsigned int c, PrintBase base) { printNumber_unsigned(c, base); }
FORCE_INLINE void print(unsigned long c, PrintBase base) { printNumber_unsigned(c, base); }


void print(EnsureDouble c, int digits) { printFloat(c, digits); }

// Forward the call to the former's method
FORCE_INLINE void print(char c) { print(c, PrintBase::Dec); }
FORCE_INLINE void print(unsigned char c) { print(c, PrintBase::Dec); }
FORCE_INLINE void print(int c) { print(c, PrintBase::Dec); }
FORCE_INLINE void print(unsigned int c) { print(c, PrintBase::Dec); }
FORCE_INLINE void print(unsigned long c) { print(c, PrintBase::Dec); }
FORCE_INLINE void print(long c) { print(c, PrintBase::Dec); }
FORCE_INLINE void print(double c) { print(c, 2); }

FORCE_INLINE void println(const char s[]) { print(s); println(); }
FORCE_INLINE void println(char c, PrintBase base) { print(c, base); println(); }
FORCE_INLINE void println(unsigned char c, PrintBase base) { print(c, base); println(); }
FORCE_INLINE void println(int c, PrintBase base) { print(c, base); println(); }
FORCE_INLINE void println(unsigned int c, PrintBase base) { print(c, base); println(); }
FORCE_INLINE void println(long c, PrintBase base) { print(c, base); println(); }
FORCE_INLINE void println(unsigned long c, PrintBase base) { print(c, base); println(); }
FORCE_INLINE void println(double c, int digits) { print(c, digits); println(); }
FORCE_INLINE void println() { write('\r'); write('\n'); }

// Default implementation for anything without a specialization
// This handles integers since they are the most common
template <typename T>
void print(T c) { print(c, PrintBase::Dec); }

void print(float c) { print(c, 2); }
void print(double c) { print(c, 2); }

void println(char *s) { print(s); println(); }
void println(const char *s) { print(s); println(); }
void println(float c, int digits) { print(c, digits); println(); }
void println(double c, int digits) { print(c, digits); println(); }
void println() { write('\r'); write('\n'); }

// Default implementations for types without a specialization. Handles integers.
template <typename T>
void println(T c, PrintBase base) { print(c, base); println(); }

template <typename T>
void println(T c) { println(c, PrintBase::Dec); }

// Forward the call to the former's method
FORCE_INLINE void println(char c) { println(c, PrintBase::Dec); }
FORCE_INLINE void println(unsigned char c) { println(c, PrintBase::Dec); }
FORCE_INLINE void println(int c) { println(c, PrintBase::Dec); }
FORCE_INLINE void println(unsigned int c) { println(c, PrintBase::Dec); }
FORCE_INLINE void println(unsigned long c) { println(c, PrintBase::Dec); }
FORCE_INLINE void println(long c) { println(c, PrintBase::Dec); }
FORCE_INLINE void println(double c) { println(c, 2); }
void println(float c) { println(c, 2); }
void println(double c) { println(c, 2); }

// Print a number with the given base
NO_INLINE void printNumber(unsigned long n, const uint8_t base) {
if (!base) return; // Hopefully, this should raise visible bug immediately

NO_INLINE void printNumber_unsigned(uint_fixed_print_t n, PrintBase base) {
if (n) {
unsigned char buf[8 * sizeof(long)]; // Enough space for base 2
int8_t i = 0;
while (n) {
buf[i++] = n % base;
n /= base;
buf[i++] = n % (uint_fixed_print_t)base;
n /= (uint_fixed_print_t)base;
}
while (i--) write((char)(buf[i] + (buf[i] < 10 ? '0' : 'A' - 10)));
}
else write('0');
}
void printNumber(signed long n, const uint8_t base) {
if (base == 10 && n < 0) {

NO_INLINE void printNumber_signed(int_fixed_print_t n, PrintBase base) {
if (base == PrintBase::Dec && n < 0) {
n = -n; // This works because all platforms Marlin's builds on are using 2-complement encoding for negative number
// On such CPU, changing the sign of a number is done by inverting the bits and adding one, so if n = 0x80000000 = -2147483648 then
// -n = 0x7FFFFFFF + 1 => 0x80000000 = 2147483648 (if interpreted as unsigned) or -2147483648 if interpreted as signed.
// On non 2-complement CPU, there would be no possible representation for 2147483648.
write('-');
}
printNumber((unsigned long)n , base);
printNumber_unsigned((uint_fixed_print_t)n , base);
}

// Print a decimal number
Expand All @@ -218,7 +238,7 @@ struct SerialBase {
// Extract the integer part of the number and print it
unsigned long int_part = (unsigned long)number;
double remainder = number - (double)int_part;
printNumber(int_part, 10);
printNumber_unsigned(int_part, PrintBase::Dec);

// Print the decimal point, but only if there are digits beyond
if (digits) {
Expand All @@ -227,7 +247,7 @@ struct SerialBase {
while (digits--) {
remainder *= 10.0;
unsigned long toPrint = (unsigned long)remainder;
printNumber(toPrint, 10);
printNumber_unsigned(toPrint, PrintBase::Dec);
remainder -= toPrint;
}
}
Expand Down
2 changes: 1 addition & 1 deletion Marlin/src/feature/tmc_util.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@
#if ENABLED(STOP_ON_ERROR)
void report_driver_error(const TMC_driver_data &data) {
SERIAL_ECHOPGM(" driver error detected: 0x");
SERIAL_PRINTLN(data.drv_status, HEX);
SERIAL_PRINTLN(data.drv_status, PrintBase::Hex);
if (data.is_ot) SERIAL_ECHOLNPGM("overtemperature");
if (data.is_s2g) SERIAL_ECHOLNPGM("coil short circuit");
TERN_(TMC_DEBUG, tmc_report_all());
Expand Down
2 changes: 1 addition & 1 deletion Marlin/src/feature/tmc_util.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ void tmc_print_current(TMC &st) {
void tmc_print_sgt(TMC &st) {
st.printLabel();
SERIAL_ECHOPGM(" homing sensitivity: ");
SERIAL_PRINTLN(st.homing_threshold(), DEC);
SERIAL_PRINTLN(st.homing_threshold(), PrintBase::Dec);
}
#endif

Expand Down
4 changes: 2 additions & 2 deletions ini/native.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,11 +120,11 @@ build_unflags = ${simulator_macos.build_unflags}
# Simulator for Windows 10
#
# MSYS2 mingw-w64-x86_64 with these packages:
# pacman -S --needed base-devel mingw-w64-x86_64-toolchain mingw64/mingw-w64-x86_64-glm mingw64/mingw-w64-x86_64-SDL2 mingw64/mingw-w64-x86_64-SDL2_net
# pacman -S --needed base-devel mingw-w64-x86_64-toolchain mingw64/mingw-w64-x86_64-glm mingw64/mingw-w64-x86_64-SDL2 mingw64/mingw-w64-x86_64-SDL2_net mingw-w64-x86_64-dlfcn
#
[env:simulator_windows]
platform = ${simulator_common.platform}
extends = simulator_common
src_build_flags = ${simulator_common.src_build_flags} -fpermissive
build_flags = ${simulator_common.build_flags} ${simulator_common.debug_build_flags} -fno-stack-protector -Wl,-subsystem,windows -ldl -lmingw32 -lSDL2main -lSDL2 -lSDL2_net -lopengl32 -lssp
build_flags = ${simulator_common.build_flags} ${simulator_common.debug_build_flags} -IC:\\msys64\\mingw64\\include\\SDL2 -fno-stack-protector -Wl,-subsystem,windows -ldl -lmingw32 -lSDL2main -lSDL2 -lSDL2_net -lopengl32 -lssp
Copy link
Contributor

@tpruvot tpruvot Aug 5, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if mingw64\include is in default include path, -ISDL2 should be enough...

Copy link
Member

@p3p p3p Aug 5, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SDL headers get installed into a subdirectory, but generally standard practice when including the header seems to be to omit it, so adding the SDL subdir as an include dir is required (at least for lib code we don't control)

This is usually solved by using the sdl2-config program to get the correct build flags, not sure how we can use that on windows from within platformios python scripts.

Copy link
Contributor

@tpruvot tpruvot Aug 5, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here ive placed them in Marlin\src\HAL\NATIVE_SIM\include\SDL2 but manually
i just added -LMarlin/src/HAL/NATIVE_SIM/lib no include... includes are prefixed by <SDL2\xxxx> in code so this path is wrong...

Marlin/src/HAL/NATIVE_SIM/lib/imgui/src/imgui_impl_sdl.cpp:55:#include <SDL2/SDL.h>
Marlin/src/HAL/NATIVE_SIM/lib/imgui/src/imgui_impl_sdl.cpp:56:#include <SDL2/SDL_syswm.h>

Copy link
Contributor Author

@sjasonsmith sjasonsmith Aug 6, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is inconvenient to have to hard-code an include path here, but I suspect this path will work for most people, and is simple to change if you need something different.

It would be better to fix it in a more robust way, but I'd rather not delay the rest of these changes to figure something out.

build_type = debug