Skip to content
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.

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

RUMBA32 Improvements and Additions #18249

Merged
merged 11 commits into from
Jun 12, 2020

Conversation

chrissbarr
Copy link
Contributor

@chrissbarr chrissbarr commented Jun 10, 2020

Description

The purpose of this PR is to improve the user experience for the various RUMBA32 boards. I have been getting a fair few user questions by email and in the RUMBA32 GitHub repo issues, and I am attempting to make changes that will address the most common issues/questions I'm seeing. There are a few common things that I think all users are having to do (enable EEPROM, get fan PWM working, etc.) that I am trying to solve here.

I do not want to be overly prescriptive/opinionated so if anyone sees a better way to make these changes or an argument against them it would be good to hear. I'm not sure what standard practice is, but I'm not in any major rush to get this merged, so it may be worth leaving as a draft for a while to see if there's any feedback.

This PR includes changes to:

  • Rename RUMBA32_AUS3D to RUMBA32_V1_0
  • Add pin definitions for RUMBA32 V1.1 under RUMBA32_V1_1
  • Add timer definitions to pins.h files to address timer conflicts in some configurations (refer to details in Fan PWM Troubleshooting / Timer Investigation Aus3D/RUMBA32#26)
  • Configure default EEPROM settings (with check to NO_EEPROM_SELECTED so user choice overrides this if specified)
    • 4kB Flash emulation with wear levelling on v1.0 and MKS boards
    • 8kB I2C EEPROM on v1.1 boards
  • Add delays for graphical LCD to Aus3D variant
    • Without this there are artefacts on graphic LCDs - MKS has this defined already
  • Enable FAN_SOFT_PWM by default for all RUMBA32.
    • PWM is not working correctly in the STM32 Arduino Core v1.7.0 that Marlin currently uses.
    • MKS are recommending in their Wiki to enable this option, and AFAIK it's currently the only workaround to get fans running correctly.

Changing Board Naming

Currently, Marlin has RUMBA32 split into two different boards - RUMBA32_AUS3D and RUMBA32_MKS. The RUMBA32_AUS3D definition is currently used for both Aus3D boards and all clones (barring the MKS boards).

Aus3D will be releasing v1.1 of the RUMBA32 board in the next month or so, and there are some minor additions required to define pins for the new version. It would be good to have a way to select the correct board (either v1.0 or v1.1). Aus3D will not be selling v1.0 anymore but there are a lot of clones on Aliexpress etc. that use this pinout + existing users.

I am proposing in this PR to rename the current RUMBA32_AUS3D board to RUMBA32_V1_0, and then creating another definition for RUMBA32_V1_1. RUMBA32_V1_0 definition will continue to support Aus3D boards and the various clone boards as it currently does. RUMBA32_V1_1 will define the new V1.1 boards from Aus3D, and if they are cloned then presumably those clones will work fine with this definition too.

I am unsure if MKS is going to make changes in future to their boards. My current thinking is that it is nice to leave the pins files separate so that they are free to do things differently if they please. The MKS boards could then have their own versions if they need to in future.

Benefits

  • Improving RUMBA32 user experience
  • Add support for upcoming RUMBA32 V1.1

Related Issues

General:
Aus3D/RUMBA32#26
Aus3D/RUMBA32#12
Aus3D/RUMBA32#21
makerbase-mks/MKS-RUMBA32#5

Fan PWM:
https://github.com/makerbase-mks/MKS-RUMBA32/wiki/About-FAN-output-voltage-2.3V
makerbase-mks/MKS-RUMBA32#11
Aus3D/RUMBA32#26
Aus3D/RUMBA32#24 (comment)
Aus3D/RUMBA32#21 (comment)

Questions to Answer

Would be good if anyone can offer input on any of these.

  • Is it safe/preferable to change the board numbering in boards.h as I've done? I've shifted several boards down to keep the numbering sequential.

Split MKS and Aus3D RUMBA32 boards to make pin definitions more readable + allow for future differences
Enable FLASH_EEPROM_EMULATION by default and enable wear levelling.
Configure timers to ensure no conflict with underlying Arduino Core
Add delays to Aus3D RUMBA32
Add RUMBA32 v1.1 pin definitions. 
Split Aus3D variants and rename.
Enable FAN_SOFT_PWM by default for RUMBA32, to overcome current issue with PWM in Arduino Core 1.7.0
@thinkyhead
Copy link
Member

  • It's fine to modify the board numbering. For some boards, the Makefile also needs an update.
  • If overriding SOFT_PWM_SCALE in some location throws a warning, it should be surrounded by #ifndef SOFT_PWM_SCALE / #endif. If the pins file very much wants to prevent certain SOFT_PWM_SCALE values it should do a sanity check. It shouldn't prevent user override from the config file, which takes precedence.

@thinkyhead
Copy link
Member

If the various rumba-32 boards have a lot in common, it's preferable to keep the *-common.h file to make that relationship clear and to highlight the differences.

@chrissbarr
Copy link
Contributor Author

Thanks for the info @thinkyhead!

If the various rumba-32 boards have a lot in common, it's preferable to keep the *-common.h file to make that relationship clear and to highlight the differences.

Fair enough, I've been a bit divided on this myself. I'll take another look at it and will revert to the shared common file.

Update Aus3D RUMBA32 test build to use new board naming.
Add second test for V1.1 board with TMC2208 driver, EEPROM, graphic LCD.
Restore common pins file for all RUMBA32 variants.
@chrissbarr
Copy link
Contributor Author

chrissbarr commented Jun 11, 2020

It's interesting that the only difference in the PlatformIO definitions for the Aus3D and MKS RUMBA32 is that the MKS boards are set to use a different USB Vendor ID (0x8000):

#
# RUMBA32 F446VE
#
[env:rumba32_f446ve]
platform      = ${common_rumba32.platform}
extends       = common_rumba32
build_flags   = ${common_rumba32.build_flags} -DUSBD_VID=0x0483

#
# MKS RUMBA32 (adds TMC2208/2209 UART interface and AUX-1)
#
[env:rumba32_mks]
platform      = ${common_rumba32.platform}
extends       = common_rumba32
build_flags   = ${common_rumba32.build_flags} -DUSBD_VID=0x8000

I don't see any way that the VID impacts the operation/use of the board, changing my (non-MKS) board here to use different VIDs has no impact on upload or USB communication. Further, it doesn't look like MKS actually own this VID.

I am tempted to combine both boards under the one PIO configuration, as that would cut down on a fair bit of duplication and make it so users don't have to pick the right PlatformIO build, just the right board in Configuration.h.

Does anyone know the history behind this separate VID or a reason that it's useful?

@thinkyhead
Copy link
Member

My assumption about the USB VID is that it refers to the USB-serial bridge interface (such as PL2303) and that it is "needed" for the host OS to know which PnP driver it needs to obtain to talk to the device. However, that's just a guess… I'm not sure why the sketch on the board needs it, or if the VID is somehow used in uploading or debugging…. It's just a value I have been trying not to change unless I knew why.

@chrissbarr
Copy link
Contributor Author

That's a pretty reasonable assumption @thinkyhead. However, there's no dedicated USB-serial bridge on the RUMBA32 boards - it's handled natively by the STM32 MCU. The MCU can be set to have any VID/PID, currently, it's set in by the Arduino Core (which gets the value from the build flags, as in the PIO config above). Further, the DFU bootloader mode has a separate hardcoded VID/PID, so upload is not related to the VID that is specified here. From the schematics and photos, USB hardware on their boards is identical to the original.

I'll leave it alone for now, but it might make for another PR in future if I can get my hands on an MKS board to test.

Update error message when using BOARD_RUMBA32 to use new names, and add error check for BOARD_RUMBA32_AUS3D.
@thinkyhead thinkyhead marked this pull request as ready for review June 12, 2020 00:24
@thinkyhead
Copy link
Member

Overall, it looks fine at this point. It can be merged pretty soon. If you have a chance to make the small change for graphical LCD, great. Otherwise I can add a follow-up patch after this is merged.

@chrissbarr
Copy link
Contributor Author

Good suggestion, that is better - I have moved the LCD defines.

I'm happy for this to be merged at this point.

Move LCD delays into #HAS_SPI_LCD block, and allow overriding by user.
@thinkyhead thinkyhead merged commit c12111e into MarlinFirmware:bugfix-2.0.x Jun 12, 2020
jmp0x0000 pushed a commit to jmp0x0000/Marlin that referenced this pull request Aug 7, 2020
HairingX pushed a commit to HairingX/Marlin that referenced this pull request Jun 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants