andrivet / ADVi3pp

ADVi3++, an alternative and better firmware for Wanhao i3 Plus printers and clones. Fork of Marlin Firmware.
https://community.advi3pp.com
GNU General Public License v3.0
243 stars 119 forks source link

Bltouch + glass bed? #166

Closed riaqn closed 6 years ago

riaqn commented 6 years ago

Feature Request

Currently the BLtouch auto leveling samples 9 points on the bed. This is quite good for plastic bed sheet which is not completely flat, but not necessarily good for glass bed which is very very flat. Therefore, instead of using 9 samples to calculate the gradient of each area in bed, for glass bed it might be better to combine the 9 samples and calculate the most accurate gradient for the whole bed.

andrivet commented 6 years ago

I do not see your point. Why an average is better than a mesh even if it is (relatively) flat? Why is it an advantage?

andrivet commented 6 years ago

Closing because:

Feel free to re-open if you want to provide the required informations

riaqn commented 6 years ago

Hello, Marlin actually provides more than one options for auto leveling, check out here: https://github.com/andrivet/ADVi3pp-Marlin/blob/a89543b609e983061155bd7ba09fa1a169a8fdbc/Marlin/Configuration.h#L900

My request is to add an option to allow users to choose from the two options, LINEAR and BILINEAR.

andrivet commented 6 years ago

You really think that I am not aware of that? Why are you asking this ("to add an option to allow users to choose from the two options, LINEAR and BILINEAR"). For me, it does not make sense.

riaqn commented 6 years ago

It makes as much sense as scientists measure a quantity several times for more accurate result. When we have a stronger assumption("the bed is flat") we can get better result. It makes sense to exploit this assumption which is true for glass bed.

However, only some of the users owns a glass bed, so it's better to add an option in the UI to allow users to specify what kind of bed are they printing on: glass or plastic.

andrivet commented 6 years ago

The goal of ADVi3++ is not to add all possible options, every possibility offered by Marlin. Marlin takes the assumption that you choose these options at compile time. So enabling 2 (non-compatible) options at compile time and selecting one or the other at runtime is complex. So unless there is a very good reason to have such options, I will not implement such complex code. If you have facts (such as a link to a document or post explaining why we need those options), I can revise my judgment. But for the moment, I will not implement what you asked. Of course, you can also make a fork and submit a Push Request.

riaqn commented 6 years ago

I think you are right - I didn't realize how complex it is to enable both compile-time options at the same time. If so, would you be so kind as to release two versions of BLtouch-enabled firmware? There will be bltouch-glass and bltouch-plastic.

If you are too busy to do so, I'd like some input from you because I'm trying to tinker the code and simply changing that macro definition seems not enough. Building it gives the following error:

Marlin/advi3pp.cpp: In member function 'void advi3pp::internals::Printer_::sensor_grid_show()':
Marlin/advi3pp.cpp:2308:50: error: 'z_values' was not declared in this scope
frame << Uint16(static_cast<int16_t>(z_values[x][y] * 1000));
^

which is obviously an auto-leveling related issue.

andrivet commented 6 years ago

For your 1st question: another .hex means another firmware to flash, test, etc. when I am doing a new release. It means also more complexity for users (I already get support questions because there are 2 .hex files). So no, unless there is a good reason.

For your 2nd question: It is a support question. Sorry, but I am doing support only on Patreon.

multiple1 commented 6 years ago

With a glas bed: An average of all 9 points makes only sense if the bed is complete level to adjust the nozzle height. The only way to reduce the leveling time and probably reducing cpu load is to reduce the amount of points to 3 (to describe a plane) if you assume that your glasbed is flat and not level.

riaqn commented 6 years ago

Obviously when I say "the average of nine points" I don't mean it literally. From the nine points we can derive several triangles whose vertices are among the nine points. The slope rate of the bed can then be calculated to be the average slope rate of the triangles.

On Sun, Sep 23, 2018, 5:23 AM multiple1 notifications@github.com wrote:

With a glas bed: An average of all 9 points makes only sense if the bed is complete level. To adjust the nozzle height.

The only way to reduce the leveling time and probably reducing cpu load is to reduce the amount of points to 3 (to describe a plane) if you assume that your glasbed is flat and not level.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/andrivet/ADVi3pp-Marlin/issues/166#issuecomment-423803028, or mute the thread https://github.com/notifications/unsubscribe-auth/ACWSVgmiuPHHF1g9zMH5VFQCqc695rZfks5ud1L_gaJpZM4WUAwM .

andrivet commented 6 years ago

@riaqn Please stop abusing this issue, this repository, my name and the name ADVi3++. Use our own repository and issues. Moreover, If are creating a fork and distribute some binary, you have to to publish your source code. What you are currently doing is a violation of the GPLv3 license. So please take the necessary steps ASAP otherwise, I will ban you.

riaqn commented 6 years ago

@andrewsil1 thanks for the kind reminder. I have published the source code here: https://github.com/riaqn/ADVi3pp-Marlin-glassbed.

As for this issue: I feel it makes sense to show people why the original author thinks it's a bad idea, or why he didn't implement it, so if you don't mind I might link to this page from my repo's README.

I feel sorry if you feel I'm abusing your name or the repo's name. I didn't intend any of that - I never claimed this fork is endorsed by you, or affiliated with to you).

riaqn commented 6 years ago

BTW, since you said not having enough time is why you didn't implement it, would it help if I make the patch compatible with the original repo so you can merge it?

andrivet commented 6 years ago

@riaqn No, I am not interested by your fork. In my point of view, it is useless. I also ask you again to stop abusing this closed issue. For example, use your own repo to distribute your binaries. Thanks.