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

[FR] Kingroon KP3 (MOTHERBOARD is MKS Robin Mini)- Suggest to consider using PIN PB2 (Reserved Connector pin) instead of PA8 (WiFI IO0 pin) for BLTouch #19392

Closed
Hokeric opened this issue Sep 14, 2020 · 14 comments · Fixed by #20158
Labels
T: Feature Request Features requested by users.

Comments

@Hokeric
Copy link

Hokeric commented Sep 14, 2020

Description

Suggest to use PIN PB2 (Reserved Connector) instead of PA8 (WiFI IO0) as defined in the pins file for BLTouch. This will have the following benefits:

  1. Enable the use of WiFi Module and BLTouch at the same time
  2. The Reserved Connector is a standard connector which is easier to connect to and connection is more secure
  3. User does not need to change the PINS file to use PB2 for every new release

Feature Workflow

  1. [First Action] Nil
  2. [Second Action] Nil
  3. [and so on...]

Additional Information

  • Provide pictures or links that demonstrate a similar feature or concept.
  • See How Can I Contribute for additional guidelines.
@Hokeric Hokeric added the T: Feature Request Features requested by users. label Sep 14, 2020
@Hokeric Hokeric changed the title [FR] Kingroon KP3 - Suggest to use PIN PB2 (Reserved Connector) instead of PA8 (WiFI IO0) for BLTouch [FR] Kingroon KP3 - Suggest to consider using PIN PB2 (Reserved Connector pin) instead of PA8 (WiFI IO0 pin) for BLTouch Sep 15, 2020
@sjasonsmith
Copy link
Contributor

There isn't enough information here to know what you need. Kingroon doesn't appear in the Configurations repo or as a board name. What MOTHERBOARD does it use?

To avoid breaking backward compatibility with existing users, you could do something like this, which enables overriding the pin from your configuration file:

#ifndef SERVO0_PIN
  #define SERVO0_PIN                          PA8 // (WiFI IO0 pin)
#endif

@sjasonsmith
Copy link
Contributor

I intended to suggest that you make the change above (or something similar) and contribute it back to Marlin through a Pull Request.

@Hokeric
Copy link
Author

Hokeric commented Sep 16, 2020

Thank you for your response. The board is MKS Robin Mini.

The change will be as follows:

Current:
#ifndef SERVO0_PIN
#define SERVO0_PIN PA8 // (WiFI IO0 pin)
#endif

After the suggested change:
#ifndef SERVO0_PIN
#define SERVO0_PIN PB2 // (signal pin of the reserved box connector)
#endif

I can do the pull request. Could you advise the procedure? I have not done it before.

@Hokeric Hokeric changed the title [FR] Kingroon KP3 - Suggest to consider using PIN PB2 (Reserved Connector pin) instead of PA8 (WiFI IO0 pin) for BLTouch [FR] Kingroon KP3 (MOTHERBOARD is MKS Robin Mini)- Suggest to consider using PIN PB2 (Reserved Connector pin) instead of PA8 (WiFI IO0 pin) for BLTouch Sep 16, 2020
@sjasonsmith
Copy link
Contributor

I would advise to leave the current pin. Changing it is going to break anybody that is already depending on the existing behavior.

Another option would be to disable both (since the board has no labeled servo connection) and require the user to specify one.
This will still break builds, but will not start using another pin which might already be used for something else.

#ifndef SERVO0_PIN
  // This board has no dedicated servo pins. Possible options include:
  // #define SERVO0_PIN PA8 // (WiFI IO0 pin)
  // #define SERVO0_PIN PB2 // (signal pin of the reserved box connector)
#endif

There are many different workflows that can be used to create a pull request. Here is one which is documented on the Marlin site:
https://marlinfw.org/docs/development/getting_started_pull_requests.html

@Hokeric
Copy link
Author

Hokeric commented Sep 21, 2020

I like the idea of giving options for the servo pin using PA8 or PB2 without enabling either one by default. Although this might break builds of users already using the PA8 pin, it can be easily fixed by simply re-enabling the PA8 pin in the pins file, or define it in the config files.

That is, I think the suggestion given above is the best solution (copied below with updates on the comments text).

#ifndef SERVO0_PIN
// This board has no dedicated servo pins. Possible options include:
// #define SERVO0_PIN PA8 // Enable BLTOUCH support on IO0 (WIFI connector)
// #define SERVO0_PIN PB2 // Enable BLTOUCH support on SIGNAL PIN of the reserved box connector (connector J11)
#endif

The PB2 pin is a better and easier solution in my opinion as it is not being for anything and the box connector is easier to connect than the WiFi connector. Therefore, it will be good as alternative to PA8.

FYI, I did previously try to re-define SERVO0 pin in my config file to use PB2 without disabling the PA8 in pins file, but somehow BLtouch did not work. I had to specifically disable the "define SERVO0_PIN PA8" for "define SERVO0_PIN PB2" to work.

@sjasonsmith
Copy link
Contributor

The #ifndef will allow it to be overridden. That was not there before.

@Hokeric
Copy link
Author

Hokeric commented Sep 21, 2020

Ah, I see. Thank you.

@Hokeric
Copy link
Author

Hokeric commented Sep 25, 2020

After some further thought, may I suggest making change in the pins file (STM32F1 MKS Robin Mini) as follows.

Before change:
#define SERVO0_PIN PA8 // Enable BLTOUCH support on IO0 (WIFI connector)

After change:
#ifndef SERVO0_PIN
#define SERVO0_PIN PA8 // Enable BLTOUCH support on IO0 (WIFI connector)
#endif

This might be the best solution as it will not break builds currently using PA8, and at the same time provide flexibility to users who want to use other pins such as PB2 for BLTOUCH without touching the pins file. Users who prefer using PB2 (or other pins) can re-define SERVO0 in one of their config files as follows which will override the definition in pins file.

#define SERVO0_PIN PB2

Thank you.

In fact, the introduction of the unconditional definition of SERV0_PIN PA8 in 2.0.6.1 caused conflict with my configuration.h file where I defined SERVO0_PIN PB2 which worked fine in 2.0.6. It took me quite a while to figure out the conflict when I found my BLTouch did not work with 2.0.6.1. Therefore, it will better if the SERVO0 pin is not enabled or only conditionally enabled by default in the pins file.

@Hokeric
Copy link
Author

Hokeric commented Nov 9, 2020

The released 2.0.7 still have this issue which breaks compilation using self-defined SERVO0_PIN.
Could you please change the SERVO0_PIN to conditional def?

Before change:
#define SERVO0_PIN PA8 // Enable BLTOUCH support on IO0 (WIFI connector)

After change:
#ifndef SERVO0_PIN
#define SERVO0_PIN PA8 // Enable BLTOUCH support on IO0 (WIFI connector)
#endif

@sjasonsmith
Copy link
Contributor

Is this the pin you are using on your board (in yellow)?
image

@Hokeric
Copy link
Author

Hokeric commented Nov 12, 2020

Is this the pin you are using on your board (in yellow)?
image

Yes, I am using this connector J11. It is pin PB2. This connector is not used for anything else so is a good/best choice for BLTouch.

@Hokeric
Copy link
Author

Hokeric commented Nov 16, 2020

Thank you very much for your help. Much appreciated.

@sjasonsmith sjasonsmith linked a pull request Nov 17, 2020 that will close this issue
@sjasonsmith
Copy link
Contributor

This is merged. Given the simplicity of the change I'll go ahead and close this issue.

@github-actions
Copy link

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.

@github-actions github-actions bot locked and limited conversation to collaborators Jan 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
T: Feature Request Features requested by users.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants