Skip to content

[2.0.x] Anycubic Trigorilla 14 pins tweak#11869

Merged
thinkyhead merged 5 commits intoMarlinFirmware:bugfix-2.0.xfrom
Palatis:bugfix-2.0.x-anycubic_trigorilla_14
Sep 27, 2018
Merged

[2.0.x] Anycubic Trigorilla 14 pins tweak#11869
thinkyhead merged 5 commits intoMarlinFirmware:bugfix-2.0.xfrom
Palatis:bugfix-2.0.x-anycubic_trigorilla_14

Conversation

@Palatis
Copy link
Contributor

@Palatis Palatis commented Sep 18, 2018

follow up #11730

  1. add missing pin definisions like FAN2_PIN and HEATER_1_PIN
  2. warn the user about the conflict between E1_CS_PIN and FAN2_PIN, instead of directly #undef them altogether.

@Palatis
Copy link
Contributor Author

Palatis commented Sep 18, 2018

I confirmed that E1 EN/DIR/STEP are correct on my board.

I connected a stepper on that and it turns (in both direction.)
However I have only one extruder so no further test is done.

Copy link
Member

@thinkyhead thinkyhead Sep 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pins files define the pins that should be used, and don't specify that a pin actually is used. So this warning will be unnecessary for the case where (1) no second fan is being used, (2) the second E plug is unused, etc. If this warning is needed, it would be best to have it in SanityCheck.h at a point where more conditions can be checked. But I'm not sure at the moment how we can determine that the CS_PIN and FAN2_PIN are definitely being used even there. We have to further check whether SPI is applicable to the selected stepper drivers.

@Palatis
Copy link
Contributor Author

Palatis commented Sep 19, 2018 via email

@thinkyhead
Copy link
Member

what do you suggest on this one, then?
strip the check altogether and rely on the "carefulness" of the user?

No. I suggested improving the sanity check.

@Palatis
Copy link
Contributor Author

Palatis commented Sep 22, 2018

what do you suggest on this one, then?
strip the check altogether and rely on the "carefulness" of the user?

No. I suggested improving the sanity check.

thanks for the explaination, English is not my native.

@Palatis Palatis changed the title Anycubic Trigorilla 14 pins tweak [2.0.x] Anycubic Trigorilla 14 pins tweak Sep 23, 2018
@thinkyhead thinkyhead force-pushed the bugfix-2.0.x branch 2 times, most recently from 9d867f9 to 849dea9 Compare September 24, 2018 03:23
@Palatis
Copy link
Contributor Author

Palatis commented Sep 24, 2018

ouch... accidently pushed the wrong branch...

@thinkyhead
Copy link
Member

Rebased and resolved conflicts. Will merge soon. To get the latest:

git checkout bugfix-2.0.x-anycubic_trigorilla_14
git fetch origin
git reset --hard origin/bugfix-2.0.x-anycubic_trigorilla_14

The added commit should produce the same results for the IS_RAMPS_EFB case, while also giving appropriate (and also matching) results for other possible configurations, such as EXTRUDERS > 1 (EEF, EEB) and TEMP_SENSOR_BED != 0 (EFB, EEB).

Palatis and others added 4 commits September 27, 2018 14:51
check them against FAN2_PIN and warn the user when conflict instead.
this way it doesn't #undef `E1_*_PIN` inside `Configuration*.h`.
as suggested by @thinkyhead, these should go into `SanityCheck.h`.
see #11869 for more details
@thinkyhead
Copy link
Member

@Palatis — You completely reverted my changes to the pins file, which were intended to provide compatibility with a wider variety of setups. Did you not follow my instructions above?

@thinkyhead
Copy link
Member

Rebased and resolved conflicts. Will merge soon. To get the latest:

git checkout bugfix-2.0.x-anycubic_trigorilla_14
git fetch origin
git reset --hard origin/bugfix-2.0.x-anycubic_trigorilla_14

The added commit should produce the same results for the IS_RAMPS_EFB case, while also giving appropriate (and also matching) results for other possible configurations, such as EXTRUDERS > 1 (EEF, EEB) and TEMP_SENSOR_BED != 0 (EFB, EEB).

@thinkyhead thinkyhead merged commit eeab414 into MarlinFirmware:bugfix-2.0.x Sep 27, 2018
@Palatis Palatis deleted the bugfix-2.0.x-anycubic_trigorilla_14 branch September 28, 2018 21:21
@Palatis
Copy link
Contributor Author

Palatis commented Oct 5, 2018

I'm very sorry about this.

I used my old "auto-rebase-everything-to-upstream" script which switch between several branches and trying to rebase them alone the upstream (in this case, the official Marlin 2.0.x branch), and then force-push to my own repo.
When the script is been written I didn't realize that others can push to the same PR branch on my own repo...

I'm really really sorry about this, please accept my apology.

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.

3 participants