grblHAL / iMXRT1062

grblHAL driver for NXP iMXRT1062 (Teensy 4.x)
Other
51 stars 39 forks source link

Update platform.ini #57

Closed Avatario34 closed 1 year ago

Avatario34 commented 1 year ago

Update platform.ini according to Terje Io's latest changes of libs to include (forks of teensy41_ethernet and uSDFS). Minor change in my_machine.h to suppress warnings during compiling.

Avatario34 commented 1 year ago

Hi, I'm still getting one compiler warning

implicit declaration of function 'rtc_get' [-Wimplicit-function-declaration] in ffutils.c, line 107 struct tm tx=seconds2tm(rtc_get());

Avatario34 commented 1 year ago

Plus another warning:

unknown option after '#pragma GCC diagnostic' kind [-Wpragmas]

in networking.c, line 86:

#pragma GCC diagnostic ignored "-Wstringop-overflow"

terjeio commented 1 year ago

implicit declaration of function 'rtc_get' [-Wimplicit-function-declaration] in ffutils.c, line 107

Adding #include "core_pins.h" fixes this for the Arduino IDE.

#if defined(__IMXRT1052__) || (__IMXRT1062__)
    #include "imxrt.h"
    #include "core_pins.h"
#else
    #include "kinetis.h"
#endif

unknown option after '#pragma GCC diagnostic' kind [-Wpragmas]

I'll recheck this - the different compilers behaviour can drive one crazy... The Arduino IDE compiler does not complain.

Minor change in my_machine.h to suppress warnings during compiling.

Do not change this file unless it is for new options. Ideally _mymachine.h should not be used for PlatformIO builds, settings should be in platformio.ini? See how it is done for this driver.

Avatario34 commented 1 year ago

I‘ll try the above corrections, thanks.

Do not change this file unless it is for new options. Ideally my_machine.h should not be used for PlatformIO builds, settings should be in platformio.ini? See how it is done for this driver.

I thought that my_machine.h is the correct place to customize your machine. Anyway, I will add this to my platform.ini and see how this works.

Avatario34 commented 1 year ago

There is another point I‘d like to mention. The Readme does have the following section in it:

Important! There is a "bug" in Teensyduino prior to v1.54 that may cause https://github.com/grblHAL/iMXRT1062/issues/6 in processing. It is possible that this is only happening when networking is enabled and then not always so. Regardless of whether networking is enabled or not it is recommended that Teensyduino v1.54 is used to build this driver. But platform.ini contains the following comment:

platform = ${common_teensy.platform} NOTE: the latest version is broken as of 2022-10-21

It compiles with Teensyduino 1.56. The current Teensyduino version is 1.57, but it does not compile with 1.57. Teensyduino 1.58 beta is in the making.

two questions/remarks:

  1. The readme needs change and should talk about using Teensyduino 1.56 rather than 1.57 (or newer)
  2. What is required to lift the code up to the most recent Teensyduino version 1.57
terjeio commented 1 year ago

I thought that my_machine.h is the correct place to customize your machine.

It normally is, but overriding _mymachine.h allows one to have preconfigured builds available from the PlatformIO command line. More work to implement though. Anyway, I prefer to leave options off in _mymachine.h to force the user to edit it before compiling. BTW the Web Builder uses PlatformIO for most builds and overrides _mymachine.h. This template is used for the iMXRT driver and options are filled out by the backend creating the binary.

The readme needs change and should talk about using Teensyduino 1.56 rather than 1.57 (or newer)

I did not write this so I have not bothered to check its validity. If incorrect it should be corrected.

What is required to lift the code up to the most recent Teensyduino version 1.57

I do not know, if there are compilation errors these should be fixed - and then the driver retested.

Generally I adhere to "if it isn't broken then do not fix it" - sometimes updating is needed to fix bugs, but no good if new ones are introduced that are hard to track down. The ESP32 IDF framework is particulary bad - too many changes between releases...

The changes fixing the warnings from _mymachine.h are ok (except about the generic board) - but not changes that enables stuff.