Fix and follow-up the PR #4955 (PINS_DEBUGGING and M43: Read pin states), etc #4974

ghost · 2016-10-08T20:42:37Z

Fix and follow-up the PR #4955 (PINS_DEBUGGING and M43: Read pin states), etc

・Fix compilation errors when PINS_DEBUGGING is enabled

error message

In file included from \AppData\Local\Temp\arduino_build_350045\sketch\Marlin_main.cpp:4667:0:

\AppData\Local\Temp\arduino_build_350045\sketch\pinsDebug.h: In function 'bool report_pin_name(int8_t)':

pinsDebug.h:39: error: expected primary-expression before 'if'

 #define IS_ANALOG(P) if ((P) >= analogInputToDigitalPin(0) && ((P) <= analogInputToDigitalPin(15) || (P) <= analogInputToDigitalPin(5)))

                      ^

\AppData\Local\Temp\arduino_build_350045\sketch\pinsDebug.h:47:7: note: in expansion of macro 'IS_ANALOG'

   if (IS_ANALOG(pin)) {

       ^

pinsDebug.h:39: error: expected ')' before 'if'

 #define IS_ANALOG(P) if ((P) >= analogInputToDigitalPin(0) && ((P) <= analogInputToDigitalPin(15) || (P) <= analogInputToDigitalPin(5)))

                      ^

\AppData\Local\Temp\arduino_build_350045\sketch\pinsDebug.h:47:7: note: in expansion of macro 'IS_ANALOG'

   if (IS_ANALOG(pin)) {

       ^

\AppData\Local\Temp\arduino_build_350045\sketch\pinsDebug.h: In function 'void report_pin_state(int8_t)':

pinsDebug.h:39: error: expected primary-expression before 'if'

 #define IS_ANALOG(P) if ((P) >= analogInputToDigitalPin(0) && ((P) <= analogInputToDigitalPin(15) || (P) <= analogInputToDigitalPin(5)))

                      ^

\AppData\Local\Temp\arduino_build_350045\sketch\pinsDebug.h:439:11: note: in expansion of macro 'IS_ANALOG'

       if (IS_ANALOG(pin)) {

           ^

pinsDebug.h:39: error: expected ')' before 'if'

 #define IS_ANALOG(P) if ((P) >= analogInputToDigitalPin(0) && ((P) <= analogInputToDigitalPin(15) || (P) <= analogInputToDigitalPin(5)))

                      ^

\AppData\Local\Temp\arduino_build_350045\sketch\pinsDebug.h:439:11: note: in expansion of macro 'IS_ANALOG'

       if (IS_ANALOG(pin)) {

           ^

・Fix compilation error when both EMERGENCY_PARSER and ULTIPANEL are enabled

error message

\AppData\Local\Temp\arduino_build_350045\sketch\Marlin_main.cpp: In function 'void gcode_M43()':

Marlin_main.cpp:4694: error: 'wait_for_user' was not declared in this scope

         wait_for_user = true;

         ^

・More "ANALOG NUMBERING" to "Analog Input"
・Add comment header to all the section of all the pins files

・Sort sections as follows:

Some special definitions (USBCON, LARGE_FLASH, etc)
Servos
Limit Switches
Z Probe
Steppers
Temperature Sensors
Heaters / Fans
Misc. Functions
LCD / Controller

・Move *_MS*_PIN away from grouping things by axis to own subsection
・Move MAX6675_SS into "Temperature Sensors" section
・Adjust spacing

Bob-the-Kuhn · 2016-10-09T00:00:16Z

Please delete the "if" line 39 in pinsDebug.h - When M43 is enabled it's stopping it from compiling.
``#define IS_ANALOG(P) if ((P) >= analogInputToDigitalPin(0) && ((P) <= analogInputToDigitalPin(15) || (P) <=analogInputToDigitalPin(5)))

Should say:
#define IS_ANALOG(P) ((P) >= analogInputToDigitalPin(0) && ((P) <= analogInputToDigitalPin(15) || (P) <= analogInputToDigitalPin(5)))

Bob-the-Kuhn · 2016-10-09T00:17:33Z

It looks like M43 W shuts everything down except thermal management. I was expecting to be able to move the motors, etc. while monitoring the pins. Just wondering if that's the expected behavior.

It would be nice if M43 output a list of the pins along with their names. I'm thinking that adding something like the following just before or after initializing the pinstate[] array would be useful:

      for (int8_t pin = first_pin; pin <= last_pin; pin++) {
        SERIAL_PROTOCOLPGM("\n");   
        report_pin_name(pin);    
        if (pin_is_protected(pin)) SERIAL_PROTOCOLPGM(" PROTECTED");
     }

ghost · 2016-10-09T04:56:17Z

delete the "if" line 39 in pinsDebug.h

That is certainly true,
if (IS_ANALOG(pin))
is expanded to
if (if ((pin) >= analogInputToDigitalPin(0) && ((pin) <= analogInputToDigitalPin(15) || (pin) <= analogInputToDigitalPin(5))))
, so maybe it's a bug. I've fixed it.

Bob-the-Kuhn · 2016-10-09T05:09:37Z

I just saw how the pin list is generated - OUCH

I was wondering why some pins came back as unused when I knew differently.

I had assumed you had a nice automated way of grabbing the names of the defined pins but it's a manually created list.

Manually maintaining the pin name list could be a real nightmare.

Out of curiosity I used grep to scan the pins files and Excel to weed out the trash to come up with a list of 176 possible pin names. It's attached if you want it.

If you want it in a different format just let me know. I can use Excel to generate the "#if ... pin_say .. #endif" sequence for this list.
pin_list.txt

ghost · 2016-10-09T05:18:15Z

By the way, I'm sorry but I'm not a member of the Marlin development team...
I'm mere contributor and mere one of users.

Bob-the-Kuhn · 2016-10-09T13:30:57Z

Same here

thinkyhead · 2016-10-09T21:04:35Z

Marlin/pins_5DPRINT.h

 #define X_STEP_PIN          0
 #define X_DIR_PIN           1
 #define X_ENABLE_PIN       23
+#define X_MS1_PIN          25
+#define X_MS2_PIN          26


I prefer to keep the microstepping pins in their own section. Moving away from grouping things by axis.

I understand it, and I've done it to other pins files also.

By the way, when contributor update the commented point, review should be closed automatically by GitHub...
But from my side (Win10 + Chrome), this review is still opened for some reason.

Bob-the-Kuhn · 2016-10-10T00:06:04Z

Here's the pinsDebug.h file with the added pin names. It cleared up some of the unused designations on my system.

It's not perfect. On my system it says that pins 66 & 67 are unused but the sensitive pins list says they're protected.

pinsDebug.h.zip

Bob-the-Kuhn · 2016-10-10T00:09:50Z

I've got my "monitor endstops & toggle LED while allowing normal system operation" routine working.

Is it appropriate to discuss it now (in this thread) or should I wait until this pull request is merged and then open up a different thread?

ghost · 2016-10-10T00:53:16Z

Is it appropriate to discuss it now (in this thread) or should I wait until this pull request is merged and then open up a different thread?

Um... I think that waiting and opening a new PR is better.
Because I've saw that altered file but I’m not sure what it means by my unperceptiveness, sorry.
Even if I integrate that altering to this PR, I can't describe about that to the development team of Marlin.

Bob-the-Kuhn · 2016-10-10T03:33:40Z

Will do.

Hope your changes go smoothly.

・More ANALOG NUMBERING to Analog Input ・Add comment header to all the section of all the pins files ・Sort sections as follows: 1. Some special definitions (USBCON, LARGE_FLASH, etc) 2. Servos 3. Limit Switches 4. Z Probe 5. Steppers 6. Temperature Sensors 7. Heaters / Fans 8. Misc. Functions 9. LCD / Controller ・Move MAX6675_SS into "Temperature Sensors" section ・Adjust spacing

ghost changed the title ~~Follow-up the PR #4955 (PINS_DEBUGGING and M43: Read pin states), etc~~ Fix and Follow-up the PR #4955 (PINS_DEBUGGING and M43: Read pin states), etc Oct 9, 2016

ghost changed the title ~~Fix and Follow-up the PR #4955 (PINS_DEBUGGING and M43: Read pin states), etc~~ Fix and follow-up the PR #4955 (PINS_DEBUGGING and M43: Read pin states), etc Oct 9, 2016

thinkyhead reviewed Oct 9, 2016

View reviewed changes

thinkyhead added PR: Improvement PR: General Cleanup PR: Bug Fix labels Oct 9, 2016

esenapaj added 2 commits October 10, 2016 14:22

Fix compilation errors

00456b0

thinkyhead merged commit a07033a into MarlinFirmware:RCBugFix Oct 10, 2016

ghost deleted the Follow-up-the-PR-#4955 branch October 10, 2016 19:40

thinkyhead mentioned this pull request Oct 24, 2016

Release 1.1.0-RC8 #5077

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix and follow-up the PR #4955 (PINS_DEBUGGING and M43: Read pin states), etc #4974

Fix and follow-up the PR #4955 (PINS_DEBUGGING and M43: Read pin states), etc #4974

ghost commented Oct 8, 2016 •

edited by ghost

Loading

Bob-the-Kuhn commented Oct 9, 2016

Bob-the-Kuhn commented Oct 9, 2016

ghost commented Oct 9, 2016

Bob-the-Kuhn commented Oct 9, 2016

ghost commented Oct 9, 2016

Bob-the-Kuhn commented Oct 9, 2016

thinkyhead Oct 9, 2016 •

edited

Loading

ghost Oct 9, 2016

ghost Oct 9, 2016 •

edited by ghost

Loading

Bob-the-Kuhn commented Oct 10, 2016

Bob-the-Kuhn commented Oct 10, 2016

ghost commented Oct 10, 2016

Bob-the-Kuhn commented Oct 10, 2016

Fix and follow-up the PR #4955 (PINS_DEBUGGING and M43: Read pin states), etc #4974

Fix and follow-up the PR #4955 (PINS_DEBUGGING and M43: Read pin states), etc #4974

Conversation

ghost commented Oct 8, 2016 • edited by ghost Loading

Bob-the-Kuhn commented Oct 9, 2016

Bob-the-Kuhn commented Oct 9, 2016

ghost commented Oct 9, 2016

Bob-the-Kuhn commented Oct 9, 2016

ghost commented Oct 9, 2016

Bob-the-Kuhn commented Oct 9, 2016

thinkyhead Oct 9, 2016 • edited Loading

Choose a reason for hiding this comment

ghost Oct 9, 2016

Choose a reason for hiding this comment

ghost Oct 9, 2016 • edited by ghost Loading

Choose a reason for hiding this comment

Bob-the-Kuhn commented Oct 10, 2016

Bob-the-Kuhn commented Oct 10, 2016

ghost commented Oct 10, 2016

Bob-the-Kuhn commented Oct 10, 2016

ghost commented Oct 8, 2016 •

edited by ghost

Loading

thinkyhead Oct 9, 2016 •

edited

Loading

ghost Oct 9, 2016 •

edited by ghost

Loading