Is it time to wean Marlin 1.1? #2797
Comments
|
I think my vote is in favor of what you suggest. But what do you mean "Delete things that are clearly 'broken' " ??? |
|
Just that. If there is some "feature" which is going to cause new users to complain "I turned XXXX on, but it doesn't work" just disable (and hide) the "turn it on switch". As an example, at some time, attempting to use mesh leveling on a delta printer was causing users to report that the nozzle was ending at the wrong position. If that is still the case, or for something similar, simply have that combination (DELTA + MESH) cause a compile error. |
|
Pleas don't delete any code. That wold be a strong affront against all of them who put their time in, and hoped to improve Marlin. |
Agreed. And that is not going to happen. But with that said... If a feature or option flat out doesn't work, it makes sense to turn it off in the released code until we can figure out how to fix it. Right? |
|
If done only in configuration files, not in the code itself I could live with: In the Configuration this is a permanent reminder to fix something. But rarely it is that simple. Mostly we have combinations off features witch do not work together (DELTA & MESH_BED_LEVELING) That's a case for sanitycheck, not for commenting out or deleting. And the there are the cases where you can't simply comment out something. Softwere-endstops and DELTA, are a not working together well. I'd say that's broken. But what they do together is just a bit more then nothing. I'd be surprised i you can find more than a few features not working in at least one configuration. In all cases where you can find a working combination it's a case for dependencies, for sanity check and or documentation, not for commenting out or deleting. Again wasting houres here, not fixing the hardware endstops |
|
@Wackerbarth Do you have a list of non-working features that are simple and straight forward to turn off? |
|
My 2 cents, just wait until the RC Is more stable. Since there are no obligations to tag the RC as a release, it's better those who want to use the RC to do so, and those who want the stable release to take that.... Honestly, just wait until the Devs says, "it's ready". There will always be issues, it's the nature of software.... |
Probably the biggest reason is that both RC-2 and RCBugFix are clearly better than the current 'Release' branch. In a sense, we are misleading people to use less stable and less functional firmware. |
|
I'm pushing for a release so that we can get rid of the, significantly inferior, 1.0.x version which is that last one released. By failing to release the parts that do work, you are depriving those users who need only those features. At the same time, you are not really helping the rest since that part of the code doesn't work anyway. As you note, there will always be "issues". But, in effect, you are saying to wait because we might fix some of them before the whole project becomes irrelevant |
Understood and is reasonable. The biggest reason to hold back is @AnHardt almost has the endstop issues resolved that keep generic, simple, vanilla Delta's from working. If there are not any other big problems lurking out there maybe we should all concentrate on getting those tested and into RCBugFix. At that point, maybe we can just change the name of RCBugFix to what ever we think is appropriate for the current, main line stable branch???? (And please notice I said Tested, and then into RCBugFix. If we were to go this route, we want RCBugFix to be very stable.) |
|
Yes, but why should that development and testing delay promotion of the current code? If something is "almost" working, it will not be difficult to add it in shortly after a release of a version that is known to have problems with "generic, simple, vanilla Delta's". Doing so doesn't really delay the code for those delta printers. But waiting does impact the non-delta printers that really don't care whether the delta code works, or not. |
|
@Wackerbarth, that is a very dangerous way of thinking... Better to always test and release rather than release and hope. I'm a developer as well and deal with huge projects. The few days to fix and test will be much less escorts and less frustration for the end user. |
|
"A few days" has turned into months..... |
|
Turning off Delta's kind of makes us look silly. And doing that might be as much work as just getting the endstops fixed for everybody. I can go with what ever everybody else agrees makes sense. My vote would be to get the endstops fixed. But either way, we should update the Release tag to point to something better. |
|
Updating the Release tag means RELEASING something. You cannot have it both ways. I agree that we need to keep working on the end stop issue and add it to the released code ASAP. Why does it "look silly" to admit that the release is only for Cartesian Bots and that we think we are "close" to releasing the Delta version (work-in-progress at ... for those wanting to experiment and help us finish getting it working)? |
@AnHardt is having a hard time making progress because of the restrictions of where and how he can do his work. That is also an example of where you cannot have it both ways. Maybe we should relax some of the constraints or help him get his stuff folded into the right place. Why don't we let @AnHardt make the call? I believe the remaining changes are very small. (I don't know that for a fact.) If he feels he can make good progress with the remaining changes maybe we can all agree to do what ever needs to be done to get them into RCBugFix quickly. Part of the reason to hold back is we don't have infinite resources and we really should verify that anything with a 'Release' status is solid. That means testing and we only have so much time. If an important (and relatively small) feature is almost done that lets us support both Cartesian and Delta printers, it makes sense (to me) to wait a week rather than cripple the release. But like I said, I can live with what ever there is broad support for doing. |
|
Having no This would prevent the goto: https://github.com/MarlinFirmware/Marlin and click the Download button users from getting the old @psavva @Roxy-3DPrintBoard |
Agreed. Let's do that.
OK. As soon as AnHardt has time to get the last remaining endstop changes in place, I'll bring it up and start testing things. With a little luck, we will be able to replace the RC branch with RCBugFix as the 'Stable Release'. |
|
@AnHardt @Roxy-3DPrintBoard -- Renaming "Release" to "1.0.2" is not appropriate. All you need to do is designate a new default branch and we can delete the "Release" branch. If you are going to designate any branch as the default, you are effectively releasing it. I cleaned up things for a potential RC-3 and posted it to MarlinDev as RC. |
|
@Wackerbarth @Roxy-3DPrintBoard The @Wackerbarth |
From a historical perspective, this is bad. It makes it very difficult for people to reconstruct things if they find they have a need to do that. Renaming it to something that conveys the message: Old_Obsolete_To_Be_Avoided_Release is better (in my mind) than deleting it. Similarly, at the same time we don't want to say "Release" yet on either RC-2 or RCBugFix. But could we add a tag that is named something that conveys the message: Most_Stable_as_of_11-30-2015_With_The_Most_Features to RCBugFix? That would steer new deployments of Marlin to be using the best code base. |
|
@Roxy-3DPrintBoard -- Deleting Release doesn't delete the already tagged 1.0.2-1 release to which it refers. The ONLY subsequent changes involve files such as the LICENSE and references in the README. There are NO differences in the sources that are fed to the compiler. If we were to release 1.1.0, then the present "Release" reference would disappear because "Release" would then point to the 1.1 series rather than the 1.0 series. This will always be the case as we move from one minor version to another. So, I see no need for any additional label on 1.0.2-1. "Old_Obsolete_To_Be_Avoided_Release" would just be silly. |
|
@AnHardt -- The purpose in linearizing the commits is so that someone can actually follow the changes which are being applied. If you fail to rebase the things that you merge in, then you end up with a commit that is virtually impossible to audit because it reaches far back in history and the meaningful changes are obscured by all of the other changes which have occurred in the interim. Unfortunately, when you have those kinds of merges, because of file overlap (particularly with the 7000+ line Marlin_main.cpp), changes that were accepted end up disappearing. Part of the cost in having things developed in parallel is that those doing the coding need to update their base reference on a regular basis. If we start splitting things up into proper modules, there will be fewer conflicts and rebasing becomes an almost trivial task. Thus, maintaining a "clean" commit stream really does have benefits. I'm sorry that you find it a problem, but the alternative is to pass the burden of reconciling changes onto everyone else just to allow you to avoid your effort. As for your reluctance to have development in another MarlinFirmware repository, you are missing the entire purpose in splitting the two views of what is really one repository. You keep insisting that DEVELOPMENT should take place in the repository that is intended to be used only for distribution. |
|
@Wackerbarth Thermal runaway is indeed still a problem; I'm running a pull from about a week ago, master branch, and still getting occasional "runaway" errors from the print bed, probably because this bed is slow to heat up (about 20 mins to reach 120 degrees). Between that and printing wide, flat parts, and with air blowing on the target area (ducted fan, roughly a 2x3 cm target around the nozzle), I can see how it could trip up the protection code. fwiw, the bed is a pre-assembled variant of the MK2 "dual power" design, running on 12v, with a soldered-on surface mount thermistor, and a sheet of borosilicate glass on top, as included with a Prusa i3 kit I built last week. In addition, I slipped a sheet of cardboard under the table as a weak insulation. I already tried fiddling with the hysteresis settings, to no avail, so as of today I've disabled the runaway protection for the bed. |
|
@VanessaE -- I don't blame you. I've tried to stay out of that part of the code and allow the original authors to refine their idea into something which actually works. But, since they seem to have lost interest in fixing the situation, I am very tempted to either remove it completely or just replace it with a very simple-minded "over-temperature" abort. I recognize that there are a few "overpowered" heated beds which can get hot enough to be a real hazard. But, for most of us, we cannot get the bed hot enough to be (excessively) dangerous. |
|
Who are the original authors of the code? I'm wondering if we should start a thread to discuss the situation and try to resolve how we are going to move forward. In the worst case, if we can't get a meaningful discussion going on the subject, with clear goals and time-frames, I think the obvious result is we do as you suggest. |
|
The problem with THERMAL_PROTECTION is not, it's not working. It's |
|
@AnHardt -- It's "not working" in the sense that a number of users who have "normal" (12v or 24v) heated beds which respond very slowly to changes have been unable to "tune" the system so that the alarm condition is not triggered during normal operation. The reality is that, for these users, they should not need to do ANY tuning. It doesn't matter if the heater is working, or not, it won't get above 125C if left "full on". The fuse in the mains is sufficient protection. For these users, they would be MUCH better off without the fancier detection algorithm (that, admittedly, has use for the hot ends and those who have high-powered heaters) |
|
Can we relax the parameters so that most people do not have a problem but still get the protection? Stepping back and looking at this from a higher level: It might be the people that added this functionality felt very strongly about the safety side of things. It is possible they set the parameters very aggressively just because they have strong feelings on the matter. I'm wondering if these issues can be cured by just backing off the numbers a little bit??? |
|
I'm unsure just what algorithm is being attempted. We really need DESCRIPTIONS of the intended behavior. "The code is the description" is NOT sufficient. As I understand it, we have a system which can have either a short or long thermal time constant and the noise on the measurement may be higher than actual "signal". I'm not convinced that "one size fits all" can work reasonably well. |
|
Who are the authors of the code? Do we know who contributed this code? |
|
It's unclear. Both Scott and Andreas have changed lines in it. But those changes might have been just cosmetic/bookkeeping rather than substantive changes to the algorithm. |
|
Given we are talking about the Release Candidate, we probably don't want to make big changes in the code anyway. Would it be acceptable to everybody to cut the thermal change in half and double the time period for the watch interval? |
|
@Roxy-3DPrintBoard has nothing to do with the bed. The equivalent for the bed was never merged. @Wackerbarth The first apperence of THERMAL_PROTECTION then THERMAL_RUNAWY_PROTECTION is older than a year for bed and nozzles, when i joined the project. The values have been then. Can't remember who, why or when the default values for TRP where changed. |
|
@Wackerbarth |
Right! Thank You! This is why discussion is important! |
You did not understand the problem. A MAXTEMP is already in - since ages. It does not help if the sensor has fallen out. Thermal runaway works only if the target temperature has been reached. The other routine is thought to cover the time, where the heater is heating to the target temperature. Please at least read the provided information in the Configuration files.
I would not exactly say I lost my interest, but as long as you destroy my PR's habitual, I will no longer contribute any PR. |
|
If anyone needs to refresh their memory on what was done, there's a whole thread on thermal protection, with lots of test results and discussion on the changes that have been made so far. The developers who worked on that did great work, and made several smart and comprehensive changes to the previous code. If there are still problems with thermal protection in the latest RC, why not revive that thread and focus resources on that one issue until it's squashed? The only caveat is that we have limited resources, and a developer can only work on one issue at a time. And there are only a few of us who have enough insight into these sections of the code to actually make the needed adjustments, if any. As for the cause, I concur that users are getting the slow-heating error due to the default settings, and not due to any inherent code problem. The defaults work for most situations, but are loath to tolerate very slow heaters, and therefore need adjustment in those cases. See the threads #1509 #1778 #1813 #2024 #2025 and especially #2066 |
|
Thanks Scott. It will take me a few days, but as I find the time, I'll read all those links you posted.
Yeah. I ran into this. I almost turned it off because I couldn't get it to behave right. But as I dug in deeper, I ended up relaxing the settings a lot. Maybe I could tighten them back up a little, but I haven't gone back and spent any time to do that. But just for the sake of discussion, what do people think the default settings should handle? I'm kind of thinking they should be set so 95% of the people don't have a problem getting the system up and working. And then maybe we should have very clear directions on how to tighten the parameters if the person's system can handle that. Does that approach make any sense? |
|
@Roxy-3DPrintBoard As I recall we shortened the time-spans so that thermal runaway and failure-to-heat conditions will be caught quickly. The old (1.0.x) timeout was something like 40 seconds, so we could certainly go back to that timespan. The documentation on the Wiki (still?) includes links to a "Thermal Runway Protection" topic, but I don't think I ever filled it out much, if at all. It should contain the comment text from the configuration file, at least, plus a good synopsis of how it operates. The "Watch Heater" function and the "Thermal Runaway Protection" feature are currently separate from one another, and must both be enabled (as they are by default) to get the fullest available protection. So the Wiki topics on these two configuration options should both refer readers to a main "Thermal Protection" topic. Aside: I see lots of new bugs being posted to the Issue queue for |
thinkyhead
added
Needs: More Data
Yes... Be careful what you wish for! Scott, if you have time, please try to bring up the Jan20 branch. You have to spend a few minutes of extra work to do that. But as you cross bug fixes over.... I would like them to land in the Jan20 branch for two reasons. First, that means you will be using that branch and can tell us about any issues that need to be addressed. But secondly, that is what I'm writing code against and I would really appreciate not fighting bugs that have been fixed! :)
I've told my printer to print all of these threads. The first one 1509 has all of the 'Old School' people in it. It has AlexBorro, Boelle, Daid, etc... And it is very clear they felt this was important and they wanted this feature to work. If for no other reason than legacy... We should make this work.** |
At this point the easiest way to do this is to fetch the latest code and the Jan20 branch in separate folders, then use a multi-file diff tool like BBEdit to compare them. It's just too much extra work to make every patch for RCBugFix, MarlinDev/dev, and Jan20 as well. I don't mind letting someone else maintain that branch. |
|
RC4 is less than 24 hours from release, with just a few minor issues and PRs that need to be followed up. It's never easy to define a "controlled" process to shake out the bugs, but an important key is to get users excited and involved in the process, following up their issues seriously, talking to them in forums, and taking the time especially to clear up our own conceptual oversights. I'm very proud of the work everyone has done to keep things moving. Over the past several weeks we've slogged through a lot of issues and patched an incredible number of glaring bugs. Sometimes we allowed in "more comprehensive" changes than usually apply in a beta cycle, and even some new features. But that's the RepRap, Maker, experimentalist, open source spirit! As new as this code is, we've done a lot of testing to make sure this will be one of the safest and most bug-free releases so far. And we continue to lay the groundwork to reform the code and bring it up to more modern standards for organization and object-oriented encapsulation. I'm very excited about the next chapter. |
|
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. |
The latest RC has been around for two months. Since few changes are occurring, and the whole purpose was to "RELEASE the part that works rather than waiting for everything to get fixed", perhaps we need to call it quits on fixing things for the 1.1.0 release, delete things that are clearly "broken", and release the rest. Then, we can address those deleted items in either 1.1.1 or 1.2.
The text was updated successfully, but these errors were encountered: