Unify all AVR90USB pin mappings #6889

thinkyhead · 2017-05-28T22:04:18Z

@Bob-the-Kuhn The idea here is to use only the Teensy mapping and say farewell to the "Marlin mapping" scheme. Does this make sense as a complete solution?

I remapped all files that were using "Marlin mapping" and labeled the pins in the other files with their ports. I took care not to change pins that were using "Non-FastIO" mapping, and I added a note to pins.h about how pins mapping works.

Note that for "Marlin" pins 34 and 35, I mapped them to Teensy pins 46 and 47 which map to ports E2 and E3. That should be 100% consistent with the way pins were getting mapped before.

Due to indentation changes, view the concise diff.

Also:

SAV-MKI pins labels seemed to indicate that most of the EXT_AUX_* pins were using the incorrect ("Marlin") mapping, while the rest seem to be using Teensy++. Patched.

Bob-the-Kuhn · 2017-05-28T23:37:48Z

The LCD pins all use FASTIO routines.

I've just pushed a commit on PR #6737 what cleans up mixing of FASTIO and non-FASTIO routines on the same pin.

As part of that I made a list of all the pins that do NOT use FASTIO. These are the pins that need to use the Teensy/MARLIN/Arduino pin map.

Signals that use analogWrite

CONTROLLER_FAN - always
SDSS - always
LCD_SDSS - always
RGB_LED_R_PIN - always
RGB_LED_G_PIN - always
RGB_LED_B_PIN - always
RGB_LED_W_PIN - always
SPINDLE_LASER_PWM_PIN - always
CASE_LIGHT_PIN - always
SERVO0_PIN, 1, 2 & 3 - always
MOTOR_CURRENT_PWM_XY_PIN - always
MOTOR_CURRENT_PWM_Z_PIN - always
MOTOR_CURRENT_PWM_E_PIN - always
FAN, FAN1 & FAN2 - only if FAST_PWM is enabled
E0_AUTO_FAN_PIN, E1, E2, E3 & E4 - only if FAST_PWM is enabled and is using FAN1_PIN
HEATER_1_PIN, HEATER_2_PIN - only if BARICUDA extruder is enabled

Bob-the-Kuhn · 2017-05-31T01:08:56Z

With your comments on my PR I'm finally realizing what this PR is about. The solution is to just to use FASTIO macros that match the Teensyduino pin map plus pins 46 & 47.

BEST SOLUTION BY FAR!!!!

I'll download this PR & start double checking the translations & run a few sanity checks. Give me a couple of days to complete that.

thinkyhead · 2017-05-31T02:47:26Z

plus pins 46 & 47

The one little tweak, which is nothing compared to all the mess we'll be fixing.

I'll download this PR & start double checking

Thanks, Bob! The only (?) part I missed and will need guidance on is how / whether we'll need to do any special remapping for pinMode, digitalWrite, or analogWrite (in those cases where we must use them — basically, whenever the pin number has to be a variable … or when it is not a fastio-based number). My understanding is that once all pins are mapped based on the DIO number, they should be suitable to pass directly to these functions or to libraries like u8g that take pins as arguments or initializers. At that point all pins would be essentially unified under the same —simple— scheme.

Roxy-3D · 2017-05-31T02:51:21Z

I'm sure this is an ignorant question. But it would be real easy to understand if we had a to_fastio(int pin) function. And a corresponding to_aurdio(int pin) function. Maybe we don't need these with this work almost done. But if we would have had those functions a year ago... Things would have been simpler.

thinkyhead · 2017-05-31T03:06:19Z

That would have been good! I think that until recently there was just a lot of confusion around the rationale for "traditional" (aka "Marlin") pin numbering and its special relationship with the AT90USB pins files, so no one wanted to mess with it. Sometimes it's hard to sort out what is just "the way it happens to be done" from "the way it must be done." Bob and I both got into the fastio weeds with M43 and lately with M3-M5 and PWM, and only recently did we start to feel confident enough to start ripping apart the pins and fastio.

Now that I've gone through the process in this PR, I am surprised how straightforward it turns out to be. At the same time, I can totally understand why no one wanted to approach it, since this is among the deepest bowels of what Marlin does.

Bob-the-Kuhn · 2017-05-31T03:10:45Z

Those are not needed with this approach. It'd be exactly like all the other CPU families - FASTIO and Arduino use the same logical pin number for the same port.

The only exception is pin 46 & 47. Those will remain FASTIO only pins just because the Arduino Teensy IDE extensions don't know what to do with them.

Bob-the-Kuhn · 2017-05-31T18:24:01Z

need to do any special remapping for pinMode, digitalWrite, or analogWrite

Once FASTIO matches the Teensy pin numbering then nothing special needs to be done. Everything would behave the same as the other CPU families with the exception of pins 46 & 47. 46 & 47 will always have to use the FASTIO routines.

thinkyhead · 2017-06-01T03:28:42Z

Overall, pretty decent for just replacing one set of numbers with another. So, does anything in this PR cause breakage? You mentioned that the LCD pins would also need to be remapped, possibly. In this PR I left all the pins named DOGM_* and LCD_PINS_* unchanged. It felt like the correct thing at the time, because I could find no instance where they were used through FastIO.

Anyway, before I dive ahead and merge this, I should look it over a couple more times to make sure I didn't miss any pins or introduce any typos. I feel pretty confident that this will work if everything is sewed up. I'm ready to breathe a sigh of relief and see an end to all our mapping woes!

Bob-the-Kuhn · 2017-06-01T09:03:05Z

My impression is that the LCD stuff is even more confusing than the FASTIO/Teensy stuff.

Using the current bugfix-1.1.x and some LEDs I found out that

DOGLCD_CS used the Teensy pin number
LCD_PINS_D4 used the FASTIO pin number

BUT... I see where LCD_PINS_D4 is passed to an Arduino library which has to be using the Teensy numbers.

As best I can tell the LCDs that use the Marlin ST7920 routines use FASTIO and everything else uses Teensy.

I'll do some more digging tomorrow.

Bob-the-Kuhn · 2017-06-01T09:14:53Z

Why are you changing all those non-AT90USB boards? I don't see any value add to adding the FASTIO flags. I do see people scratching their heads wondering what it's about.

If my notes from 6 months ago are correct then the only pins_XXX.h files that need to be touched are:

pins_5DPRINT.h			AT90USB1286
pins_BRAINWAVE_PRO.h			AT90USB1286
pins_PRINTRBOARD.h			AT90USB1286
pins_PRINTRBOARD_REVF.h			AT90USB1286
pins_SAV_MKI.h			AT90USB1286
pins_TEENSY2.h			AT90USB1286
pins_TEENSYLU.h			AT90USB1286
pins_BRAINWAVE.h			AT90USB646

Bob-the-Kuhn · 2017-06-02T00:03:58Z

Progress report

I've put more LEDs on my Teensylu and downloaded all the LCD controllers that use the LCD_PINS_xx pins and DOGLCD_xx pins.

I've confirmed that all the DOGLCD_xx pins are FASTIO.

I've confirmed that the LCD_PINS_xx are a mix. 7 of the 9 controllers that use these signals use the FASTIO pins. REPRAP_DISCOUNT_FULL_GRAPHIC_SMART_CONTROLLER and BQ_LCD_SMART_CONTROLLER use the Teensy pins.

Only the Printrboard files are affected by the mix. Unfortunately they do NOT have tight enough if-then-else defines to tell the difference. The same #define LCD_PINS_xx results in different pins being used. No matter which way we go we'll be causing problems for someone.

CARTESIO_UI					FASTIO DOGLCD_xx
ELB_FULL_GRAPHIC_CONTROLLER			FASTIO DOGLCD_xx
MAKRPANEL					FASTIO DOGLCD_xx
MINIPANEL					FASTIO DOGLCD_xx
miniVIKI					FASTIO DOGLCD_xx
VIKI2						FASTIO DOGLCD_xx

REPRAP_DISCOUNT_SMART_CONTROLLER		FASTIO LCD_PINS_xx
G3D_PANEL					FASTIO LCD_PINS_xx
PANEL_ONE					FASTIO LCD_PINS_xx
RIGIDBOT_PANEL					FASTIO LCD_PINS_xx
ULTIMAKERCONTROLLER				FASTIO LCD_PINS_xx
ULTIPANEL					FASTIO LCD_PINS_xx
REPRAPWORLD_GRAPHICAL_LCD			FASTIO LCD_PINS_xx

BQ_LCD_SMART_CONTROLLER				Teensy LCD_PINS_xx
REPRAP_DISCOUNT_FULL_GRAPHIC_SMART_CONTROLLER	Teensy LCD_PINS_xx

Nest step is to do the same thing with the I2C controllers and the shift register controllers.

thinkyhead · 2017-06-02T01:21:16Z

Why are you changing all those non-AT90USB boards?

Only added commentary to them, trying to clarify which pins are non-FastIO in all pins files.

thinkyhead · 2017-06-02T01:30:22Z

I've confirmed that all the DOGLCD_xx pins are FASTIO.

Ok, removed the "non-FastIO" comments from all of those.

thinkyhead · 2017-06-02T01:38:10Z

BQ_LCD_SMART_CONTROLLER				Teensy LCD_PINS_xx
REPRAP_DISCOUNT_FULL_GRAPHIC_SMART_CONTROLLER	Teensy LCD_PINS_xx

So it looks like if I re-map the LCD_PINS_* numbers for these displays only, then all pins will be unified under "FastIO" mapping.

Now, when you say "Teensy" mapping in the context of these displays, do you mean they are already mapped to the Teensy DIO pin numbers, and therefore do not need to be changed?

Or, are you indicating the LCD_PINS_* for these LCDs are using the "Marlin" pin numbers that we were using for some of the AT90USB boards, and which became associated with Teensy, and so they do need to be changed?

The thing is, "FastIO" and "Teensy" are not the opposite of each other. "Marlin" and "Teensy" are the two competing pin mappings. "FastIO" as I said before applies always. So, should the above really read like this…?

CARTESIO_UI                                     "Marlin" DOGLCD_xx
ELB_FULL_GRAPHIC_CONTROLLER                     "Marlin" DOGLCD_xx
MAKRPANEL                                       "Marlin" DOGLCD_xx
MINIPANEL                                       "Marlin" DOGLCD_xx
miniVIKI                                        "Marlin" DOGLCD_xx
VIKI2                                           "Marlin" DOGLCD_xx

REPRAP_DISCOUNT_SMART_CONTROLLER                "Marlin" LCD_PINS_xx
G3D_PANEL                                       "Marlin" LCD_PINS_xx
PANEL_ONE                                       "Marlin" LCD_PINS_xx
RIGIDBOT_PANEL                                  "Marlin" LCD_PINS_xx
ULTIMAKERCONTROLLER                             "Marlin" LCD_PINS_xx
ULTIPANEL                                       "Marlin" LCD_PINS_xx
REPRAPWORLD_GRAPHICAL_LCD                       "Marlin" LCD_PINS_xx

BQ_LCD_SMART_CONTROLLER                         "Teensy" LCD_PINS_xx
REPRAP_DISCOUNT_FULL_GRAPHIC_SMART_CONTROLLER   "Teensy" LCD_PINS_xx

Marlin: Pin 0 = A0
Teensy: Pin 0 = D0

thinkyhead · 2017-06-03T06:11:19Z

Marlin/pins_BRAINWAVE.h

@@ -82,7 +82,7 @@
 #define X_STEP_PIN          3   // D3
 #define X_DIR_PIN           5   // D5
 #define X_ENABLE_PIN        4   // D4
-#define X_ATT_PIN           3   // D3
+#define X_ATT_PIN           2   // D2


Has this been wrong till now?

Bob-the-Kuhn · 2017-06-03T17:41:31Z

Yes, remove all mentions of FASTIO , non-FASTIO, Teensy , .. from the pin comments.

Here's the updated Brainwave PRO file. It originally used the Teensy system so I took your conversion, checked it, added info on the expansion connectors and updated the header.

On the Brainwave question - for X_ATT_PIN pin 2 port D2 is correct.

Bob-the-Kuhn · 2017-06-05T16:03:53Z

@fiveangle has tested the Printrboard Rev F. He's pointed out two things:

The FAN_PIN should not change. It should always be assigned to the dedicated port. That makes sense. The original Printrboard file had definite problems with the fan. I think that Dave's request implements the intention of the original file.
Teensyduino can now download images bigger than 64K so there isn't much need for the Printrboard IDE extension. My Teensylu doesn't have a Teensyduino compatible bootloader on it so I'm stuck with Printrboard.

I've changed the fan per Dave's request.

I've also modified the text to indicate that Teensyduino is the most used IDE extension and removed the comment about the 64k limit.

Here's the modified files. Brainwave didn't change but I included it for completeness.

With Dave's testing I'm comfortable merging this PR with the updated pins_XXX.h files.

fiveangle · 2017-06-06T03:42:36Z

Noble work here @Bob-the-Kuhn. With the weath of misinformation about all this spread out over the years, this was a long time coming and the Marlin world solutes you for the perseverance :)

Bob-the-Kuhn · 2017-06-06T04:04:20Z

Scott was the one that got the courage up to do away with the old system.

I'm REALLY looking forward to seeing it merged.

Bob-the-Kuhn · 2017-06-07T21:26:37Z

@thinkyhead - Scott what are your plans for this PR? Is there anything I can do to get it ready to merge?

Bob-the-Kuhn · 2017-06-07T21:35:57Z

I see the non-AT90USB boards are still in the PR. I really believe that adding non-FASTIO to them is of no value and will result in questions.

thinkyhead · 2017-06-08T23:10:12Z

Just had a chance to get it cleaned up. Please give it another review!

Bob-the-Kuhn · 2017-06-09T15:53:53Z

Please replace the AT90USB boards with these files

The comments have been changed to say that the main reason for using Printrboard IDE extension is if the board doesn't have the Teensyduino bootloader. The comment that implied that Teensyduino can't upload images greater than 64k has been removed because it is no longer true.

The fan pin in the Printrboard files have been corrected.

Once the files have been updated then this PR is ready to merge.

thinkyhead · 2017-06-09T23:32:21Z

Marlin/pinsDebug.h

    }
-    //if (!pwm_status(pin)) SERIAL_CHAR(' ');    // add padding if it's not a PWM pin
-    if (extended) pwm_details(pin);  // report PWM capabilities only if doing an extended report
-    SERIAL_EOL();


@Bob-the-Kuhn I had a little hiccup when merging, wasn't sure how the end of this function should be. Are these changes the same as they were?

That is correct. The SERIAL_EOL has been moved a couple of levels up/left.

thinkyhead · 2017-06-09T23:49:02Z

Ok, I've brought in your changes. Looks like not a lot needed adjustment. Could it be this is ready to merge??

Bob-the-Kuhn · 2017-06-10T00:30:28Z

Most definitely it's ready

thinkyhead · 2017-06-10T20:31:04Z

So, in layman's terms how should we hype this?

"Easier to build Marlin for Printrboard, Teensy++, and other AT90USB-based boards!"

Does that capture everything?

Bob-the-Kuhn · 2017-06-10T22:03:40Z

For the user it's "Easier to customize and to understand."

thinkyhead added PR: General Cleanup PR: Improvement labels May 28, 2017

thinkyhead force-pushed the bf_at90usb_mapping branch 7 times, most recently from 1d26339 to ffa7a93 Compare May 28, 2017 23:16

thinkyhead force-pushed the bf_at90usb_mapping branch from ffa7a93 to d12bf22 Compare May 29, 2017 00:10

This was referenced May 29, 2017

See PR #6889 for a better solution (AT90USB - Teensyduino IDE compatibility (USBCON issue fixed on another PR)) #6737

Closed

See PR #6889 for a better solution (AT90USB - use only Teensy pin numbers in pins_XXX.h files) #6896

Closed

thinkyhead force-pushed the bf_at90usb_mapping branch 2 times, most recently from c562706 to 837688b Compare May 30, 2017 20:11

Bob-the-Kuhn mentioned this pull request Jun 1, 2017

compile options for 5Dprint & Printrboards #5708

Closed

thinkyhead force-pushed the bf_at90usb_mapping branch from 837688b to d1199d0 Compare June 2, 2017 01:28

thinkyhead force-pushed the bf_at90usb_mapping branch 2 times, most recently from 835d5dc to 36fa6ae Compare June 3, 2017 05:45

thinkyhead commented Jun 3, 2017

View reviewed changes

thinkyhead force-pushed the bf_at90usb_mapping branch 2 times, most recently from fa7c24c to c757d95 Compare June 8, 2017 23:08

thinkyhead force-pushed the bf_at90usb_mapping branch from c757d95 to bf82e69 Compare June 8, 2017 23:19

Unify AVR90USB: fastio changes

b378dea

thinkyhead force-pushed the bf_at90usb_mapping branch from 8f736f5 to 44bec97 Compare June 9, 2017 23:28

thinkyhead commented Jun 9, 2017

View reviewed changes

thinkyhead added 2 commits June 9, 2017 18:48

Unify AVR90USB: Pins remap to Arduino

87d8223

Unify AVR90USB: Pins cleanup

5426fc1

thinkyhead force-pushed the bf_at90usb_mapping branch from 44bec97 to 1bec42b Compare June 9, 2017 23:48

thinkyhead added 2 commits June 9, 2017 19:20

Unify AVR90USB: pinsDebug changes

748bf32

Save 8 bytes of SRAM

51bc502

thinkyhead force-pushed the bf_at90usb_mapping branch from 1bec42b to 51bc502 Compare June 10, 2017 00:20

thinkyhead merged commit f17a3c2 into MarlinFirmware:bugfix-1.1.x Jun 10, 2017

thinkyhead deleted the bf_at90usb_mapping branch June 10, 2017 02:00

thinkyhead mentioned this pull request Jul 22, 2017

[1.1.x] Update pin definitions for PRINTRBOARD REV F. #7286

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Unify all AVR90USB pin mappings #6889

Unify all AVR90USB pin mappings #6889

thinkyhead commented May 28, 2017 •

edited

Loading

Bob-the-Kuhn commented May 28, 2017

Bob-the-Kuhn commented May 31, 2017

thinkyhead commented May 31, 2017 •

edited

Loading

Roxy-3D commented May 31, 2017 •

edited

Loading

thinkyhead commented May 31, 2017

Bob-the-Kuhn commented May 31, 2017

Bob-the-Kuhn commented May 31, 2017

thinkyhead commented Jun 1, 2017

Bob-the-Kuhn commented Jun 1, 2017

Bob-the-Kuhn commented Jun 1, 2017

Bob-the-Kuhn commented Jun 2, 2017

thinkyhead commented Jun 2, 2017

thinkyhead commented Jun 2, 2017

thinkyhead commented Jun 2, 2017 •

edited

Loading

thinkyhead Jun 3, 2017

Bob-the-Kuhn commented Jun 3, 2017

Bob-the-Kuhn commented Jun 5, 2017

fiveangle commented Jun 6, 2017

Bob-the-Kuhn commented Jun 6, 2017

Bob-the-Kuhn commented Jun 7, 2017

Bob-the-Kuhn commented Jun 7, 2017

thinkyhead commented Jun 8, 2017

Bob-the-Kuhn commented Jun 9, 2017

thinkyhead Jun 9, 2017 •

edited

Loading

Bob-the-Kuhn Jun 9, 2017

thinkyhead commented Jun 9, 2017

Bob-the-Kuhn commented Jun 10, 2017

thinkyhead commented Jun 10, 2017 •

edited

Loading

Bob-the-Kuhn commented Jun 10, 2017

Unify all AVR90USB pin mappings #6889

Unify all AVR90USB pin mappings #6889

Conversation

thinkyhead commented May 28, 2017 • edited Loading

Bob-the-Kuhn commented May 28, 2017

Bob-the-Kuhn commented May 31, 2017

thinkyhead commented May 31, 2017 • edited Loading

Roxy-3D commented May 31, 2017 • edited Loading

thinkyhead commented May 31, 2017

Bob-the-Kuhn commented May 31, 2017

Bob-the-Kuhn commented May 31, 2017

thinkyhead commented Jun 1, 2017

Bob-the-Kuhn commented Jun 1, 2017

Bob-the-Kuhn commented Jun 1, 2017

Bob-the-Kuhn commented Jun 2, 2017

thinkyhead commented Jun 2, 2017

thinkyhead commented Jun 2, 2017

thinkyhead commented Jun 2, 2017 • edited Loading

thinkyhead Jun 3, 2017

Choose a reason for hiding this comment

Bob-the-Kuhn commented Jun 3, 2017

Bob-the-Kuhn commented Jun 5, 2017

fiveangle commented Jun 6, 2017

Bob-the-Kuhn commented Jun 6, 2017

Bob-the-Kuhn commented Jun 7, 2017

Bob-the-Kuhn commented Jun 7, 2017

thinkyhead commented Jun 8, 2017

Bob-the-Kuhn commented Jun 9, 2017

thinkyhead Jun 9, 2017 • edited Loading

Choose a reason for hiding this comment

Bob-the-Kuhn Jun 9, 2017

Choose a reason for hiding this comment

thinkyhead commented Jun 9, 2017

Bob-the-Kuhn commented Jun 10, 2017

thinkyhead commented Jun 10, 2017 • edited Loading

Bob-the-Kuhn commented Jun 10, 2017

thinkyhead commented May 28, 2017 •

edited

Loading

thinkyhead commented May 31, 2017 •

edited

Loading

Roxy-3D commented May 31, 2017 •

edited

Loading

thinkyhead commented Jun 2, 2017 •

edited

Loading

thinkyhead Jun 9, 2017 •

edited

Loading

thinkyhead commented Jun 10, 2017 •

edited

Loading