MarlinFirmware / Marlin

Marlin is an optimized firmware for RepRap 3D printers based on the Arduino platform. Many commercial 3D printers come with Marlin installed. Check with your vendor if you need source code for your specific machine.
https://marlinfw.org
GNU General Public License v3.0
16.32k stars 19.25k forks source link

[BUG] Bad fan control on MKS Monster8 V1.0 #24080

Closed rondlh closed 2 years ago

rondlh commented 2 years ago

Did you test the latest bugfix-2.0.x code?

Yes, and the problem still exists.

Bug Description

I have a part cooling fan (IO P2.4) and hotend fan (IO P1.4) connected on an MKS Monster8 V1.0 (STM32F406VET6 cpu). Both fans are not working well. The hotend fan starts but repeatedly stops for about half a second every 5 seconds, and the part cooling fan PWM doesn't work, it seems more a binary control, full speed or something slow. I manage to find a solution/workaround:

By default (MARLIN_F4x7Vx\variant.h):

#define TIMER_TONE            TIM6
#define TIMER_SERIAL          TIM5

The fans work fine if I swap these timers around.

Bug Timeline

Don't know

Expected behavior

Proper fan operation, PWM fan control

Actual behavior

Inconsistent hotend fan behavior (hiccups), incorrect PWM fan control (kind of binary)

Steps to Reproduce

  1. MKS Monster8 V1.0 board
  2. Use standard Marlin configuration, enable a part cooling fan (IO P2.4) and hotend fan (IO P1.4).
  3. Heatup the hotend above 50 degrees C to start the hotend fan, try to control the part cooling fan over the whole range.

Version of Marlin Firmware

Marlin 2.0.9.3 latest bugfix

Printer model

Custom

Electronics

MKS Monster8 V1.0

Add-ons

BTT TFT35 V3.0

Bed Leveling

No response

Your Slicer

No response

Host Software

No response

Additional information & file uploads

No response

dckostin commented 2 years ago

I have the same issue with the print cooling fan not running at all. The print cooling fan always running ([BUG] Marlin 2.0.9.3 cooling fan #23418) "fix" in the bugfix-2.0 branch stopped the fan from always running and now will not start at all. I am running a Fysetc Spider v2.2 controller board with a mini12864 display. An artifact of either this bugfix or something else in the bugfix-2.0 branch has made it such that the mini12864 display light will not turn on. I have cycled between the release and bugfix branches multiple times and confirmed the print cooling fan not running (and the display light not turning on) are associated with the bugfix-2.0 branch. Unfortunately, I am not proficient enough in GitHub to determine exactly which commit resulted in this behavior.

thinkyhead commented 2 years ago

Sorting out timer conflicts is always fun. We can look at other STM32F4-based boards with similar pinouts and see what timers they are using.

rondlh commented 2 years ago

It could also be a timer assignment issue, you could give a try modifying: buildroot\share\PlatformIO\variants\MARLIN_F446VE\variant.h

Have a look at this section:

// Timer Definitions
// Use TIM6/TIM7 when possible as servo and tone don't need GPIO output pin
#ifndef TIMER_TONE
  #define TIMER_TONE            TIM6  // TIMER_TONE must be defined in this file
#endif

#ifndef TIMER_SERVO
  #define TIMER_SERVO           TIM7  // TIMER_SERVO must be defined in this file
#endif

#ifndef TIMER_SERIAL
  #define TIMER_SERIAL          TIM9  // TIMER_SERIAL must be defined in this file
#endif
dckostin commented 2 years ago

Thank you for your help! So I'm understanding that you're looking at "STM32 ARM Cortex-M4F" boards because that is the overlap for both the MKS_MONSTER8 and the FYSETC_SPIDER_V2_2, and why others are not having this issue. Smart! I have the section of the header file. What change would you like for me to make to run the test?

rondlh commented 2 years ago

I just noticed something odd, in stm32f4.ini it says:

board_build.variant = MARLIN_F4x7Vx

board                       = marlin_STM32F407VGT6_CCM IRON
build_flags                 = ${stm32_variant.build_flags} ${stm32f4_I2C1_CAN.build_flags}
                              -DHAL_PCD_MODULE_ENABLED -DTIMER_SERIAL=TIM4

So TIMER_SERIAL=TIM4. Actually the Monster8 has a STM32F407VET6 (both according to the provided schematic and the 2 boards I have). https://github.com/makerbase-mks/MKS-Monster8/blob/main/hardware/MKS%20Monster8%20V1.0_003/MKS%20Monster8%20V1.0_003%20SCH.pdf

In MARLIN_F4x7Vx\variant.h it says:

#ifndef TIMER_SERIAL
  #define TIMER_SERIAL          TIM5 // TIMER_SERIAL must be defined in this file
#endif

If I change this to TIM4 then my problem is gone.

rondlh commented 2 years ago

Thank you for your help! So I'm understanding that you're looking at "STM32 ARM Cortex-M4F" boards because that is the overlap for both the MKS_MONSTER8 and the FYSETC_SPIDER_V2_2, and why others are not having this issue. Smart! I have the section of the header file. What change would you like for me to make to run the test?

I'm not sure what to advice you, you could just play around with it a bit and see if it compiles, and if it solves your problem. You could try with TIM6 and TIM9 swapped, or change TIMER_SERIAL to TIM2 or TIM4

dckostin commented 2 years ago

Thank you both for the guidance on troubleshooting the timer assignments! Although the Fysetc Spider has a STM32F446VE, it appears to be using "buildroot\share\PlatformIO\variants\MARLIN_FYSETC_S6\variant.h" file which says:

define TIMER_SERIAL TIM7 // TIMER_SERIAL must be defined in this file

By changing this to TIM4 like rondlh had done, this also resolved my problem!

Should this change be implemented in github and if so, who can do this?

thisiskeithb commented 2 years ago

Should this change be implemented in github and if so, who can do this?

Someone will need to verify that the change won't interfere with other enabled/possibly enabled features or we'll continue to chase this timer conflict issue.

dckostin commented 2 years ago

I'm willing to do the verify activity it someone is willing to tell me what I would need to do.

rondlh commented 2 years ago

@dckostin You would need to test the settings on all motherboards with the same micro controller, which obviously nobody can do. So you reporting back that it works for you is already very helpful.

Timers are linked to specific IO pins, which is key for PWM pins to work. So this means that the motherboard pinout affects which timer assignment should be used. This might be different for every motherboard.

Just my 2 cents on this topic... It would be nice if this timer assignment could be part of the pin definition, because there is a pin definition for every motherboard. Then the correct timer assignment would just be added there and other motherboards are not affected. I tried this, but it didn't work, it seems that the pins file is included too late in the compilation process. Then there are the definitions in the ini files. This helps, but the timer assignment is not carried over to the whole complication process (TIMER_SERIAL=TIM4)

(build_flags= ${stm32_variant.build_flags} ${stm32f4_I2C1_CAN.build_flags}
                        -DHAL_PCD_MODULE_ENABLED -DTIMER_SERIAL=TIM4)

So the variant files come into play. There it is tried to cover specific micro controller models, perhaps a special case can be added there, or a specific motherboard can be addressed (e.a. MARLIN_BIGTREE_OCTOPUS_V1).

dckostin commented 2 years ago

@rondlh, Thanks for the explanation from which I was able to research the STM32F446xx CPU. It turns out to be a many-to-many complexity as the timers are able to be assigned to multiple pins, so the research would be to ensure that at least one pin is available for each timer being assigned; in my Fysetc Spider case it's three timers (per timers_in_use) and 2 to 9 pin options per timer. There is a FYSETC_S6 variant.h file, so at least the number of boards is somewhat constrained.

Strikes me as your pin definition location idea is the way to go, but I imagine the complexity involved in moving the pins files earlier in the compile must have been such that the variants.h files were created instead.

Another option could be to follow the pins model and create a FYSETC_SPIDER variant.h with the timer definition:

image

Then put something like this in the current FYSETC_S6 variant file:

ifndef TIMER_SERIAL

define TIMER_SERIAL TIM7

endif

Another change is also required wherever the logic is located that determines which variant file to include.

rondlh commented 2 years ago

@thinkyhead The status of this issue says "Needs more date", anything I can do?

dckostin commented 2 years ago

I do not need any more data. Changing my TIMER_SERIAL from TIM7 to TIM4 resolved my issue in the bugfix-2.0 branch. What was interesting was when this fix was incorporated into the main branch, 2.0.x, my printer started working correctly using TIM7. I’m not the expert, but this may mean the need to research the other processor boards that used this same variant is no longer needed?

thinkyhead commented 2 years ago

Three boards would be affected by the change to env:rumba32, so the safe way to proceed is to create a new variant and a new env: that specifies the new variant, and just apply it to the board(s) verified here to have been fixed by the change. That much we can do without more data, but we just haven't gotten around to it yet, with all apologies.

github-actions[bot] commented 2 years ago

This issue has had no activity in the last 60 days. Please add a reply if you want to keep this issue active, otherwise it will be automatically closed within 10 days.

github-actions[bot] commented 2 years ago

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.