I found this when I tried to change it and found it had a max of 3200 then flips to 5 and no way to jog back to 3200 and I use 4800 steps/mm.
I reproduced the issue.
The problem is that EDIT_QSTEPS
in menu_advanced.cpp configures a max range of 9999, but underlying implementation in MenuItemBase::edit can only handle encoder values within the range of an int16_t. A scaling factor of 10 is used for this value type, so it wraps to zero when when you increase past 3276.8.
If I modify EDIT_QSTEPS to a max range of 2000, the menu behaves exactly as expected, with it stopping at the min and max values.
Hopefully this can be modified to allow the range specified, but it would be nice to at least error at compile time if the menu item's configuration exceeds the encoder range.
The other thing is not only get that to 32 bit for we with 32 bit boards but also make it so if we go over the max value and shoves us back at 5 (the min value) we can go backwards and get back to the max so it is circular. As it is now it is a PITA and doesn't work for anyone that has modded their machines. A lot of people did but they may still be using something else as the controlling software.
It isn't supposed to wrap around like that. As I mentioned, it stops at 2000 when I set that at a limit. It is only wrapping back to zero because it is overflowing the 16-bit integer before it hits the maximum.
I suspect a 16-bit integer was used for the encoder, because elsewhere it is used as a float, and casting from a 32-bit integer to a float is a lossy conversion.
I think my other issues is related to this and I am going to test in a few minutes.
Yep, I just tested and the 4800 steps/mm is the culprit and when I go to print nothing but I changed it to 2400 and Z was engaged and moved as it should so both of these are interrelated so it isn't just the LCD menu affected by this as I can't use Marlin at all thanks to this.
@DarkAlchy Is that with the EEPROM enabled?, if so try without.
EEPROM was off and tried it on but this is a code issue, as sjasonsmith said. The Marlin team needs to fix this as it is beyond my ability to do so especially since this issue is affecting not only the LCD panel but printing at all if you go over that 3200 steps/mm.
My first question would be "why do you need 4800 steps/mm"? That is a very high number of steps. I'm not doubting that there is a problem with Marlin, but even it that problem gets fixed, there will be some other limit (that's just the nature of working with systems that have limited capabilities, it is not practical to support a massive range of steps/mm along with a sensible range of speeds etc.). With that number of steps I suspect that you are going to start hitting other problems (like being able to generate step pulses fast enough to have sensible speeds). So although it may be possible for Marlin to fix this problem a better solution may be for you to set things up so that you need less steps/mm. What exactly is it that needs such a large number? Do you really need something that can move in increments of 0.0002mm? I suspect that most mechanical systems will be struggling with that.
My first question would be "why do you need 4800 steps/mm"?
The simple answer is it is a one motor one belt design on the Huge TronXY builds for the 330/400/500 square mm build plate. 16t on the motor and 48t on the 2mm per rev lead screws. Original was two motors on 8mm per rev for 400 steps/mm. New way has a multiplier of 12 (x4 for 2mm vs 8mm rev and x3 for the gearing for torque issues) so 4800 steps per mm. Repetier firmware had no issues with the 4800 but Marlin does.
Why not just reduce the microsteps on that motor? Assuming you were using 16 microsteps with the 8mm leadscrews (for 400 steps/mm) you could go down to 2 microsteps with your new configuration and end up with 600 steps/mm.
I never liked how that sounded as the 16 just sounds smooth and don't be fooled my Z can do 30mm/s easily on the old Melzi that had Repetier on it.
Considering that Marlin is using an Int then casts it into a 32bit I am not sure why the 16 was used at all? It is that Integer 16bit that is the culprit.
It is not my call but I really doubt if this will get fixed anytime soon. You have already discovered that the problem goes deeper than just the LCD setting problem. Given that the current focus is on getting Marlin 2.0.x stable and released and that most Marlin users do not operate with this kind of steps/mm I doubt if anyone will want to start making the sort of (potentially big) changes that may be needed to completely fix the problem (it is very likely that fixing one issue will simply expose another, that tends to be how overflow problems like this work), but who knows.
Although this is not the ideal solution I'd be looking at other ways to make the problem go away. What drivers do you use? If you can use a TMC driver you could use lower microsteps with interpolation and perhaps still get smooth operation?
@gloomyandy It is what I figured but the thing is if they push it under a rug now it will never ever be taken care of. When a major change happens is when it needs to be tackled because the devs will have less balls to do so as time marches on after the initial release. I would love to know whose brilliant idea it was to use an Int_16 and cast it over to a 32bit instead of just using 32 bit? Find the dev that did that imo. I say this because when Repetier works and is on a crappy 8bit 1284p and handles it just fine and here is a sparkling brand new Marlin that can't that really does say something.
You have already discovered that the problem goes deeper than just the LCD setting problem.
The particular int16_t issue I found does not go deeper than the LCD menu. I replied to the other issue DarkAlchy opened that I've reconfigured my printer to use 256 microsteps so I could get up to 6400 steps/mm, and it is still working fine with the latest state of 2.0. I also verified I could save large values to the EEPROM and persist them past a power cycle.
I looked around to see if I could find anywhere else that might be casting the value to an int16_t, and it seems that everywhere else handles it as a float or a uint32_t. It is possible there is _something_ I'm missing that doesn't impact my printer, but it seems limited to the menu implementation to me.
Lots of printers in the examples use 4000+ steps/mm on the Z, so I suspect more people would be having the issue if actual movement were broken. I doubt many people configure XYZ steps through the LCD, so few people would be impacted by the menu bug.
**I suggest scoping this as an actual bug, but only for the LCD menu items not allowing floats larger than 3276.8. I hope the details I have provided are sufficient to confirm there is a bug in the menu.
Anything related to actual Z axis movement seems likely to be an unrelated configuration issue, and should be dealt with in DarkAlchy's other issue.**
I'm willing to look into addressing the menu issue, but I won't be able to look at it further for at least a week.
DarkAlchy, you need to remember that this is not a commercial product you are paying for. A lot of the people contributing to it are not compensated in any way, meaning you can't really demand anything of them. Honestly, I don't care whether this menu item works. I never even knew it existed before yesterday, and would never even consider using it. I'm just interested in contributing to help make Marlin better, because I appreciate the work _other_ people have done in this community.
I spent time looking at it to help give back the community, and more specifically to help _you_ in particular. Politeness goes a lot further than insults in encouraging people to want to help. I would hate to see this very real bug be closed because people were tired of interacting with you.
I suspect there are some legit reasons for handling the encoder values the way it is right now, which is why I didn't post a quick fix for it last night. The "obvious" quick fixes may have unintended consequences for other menu items which relied on that int16_t cast.
I feel like I've gone above and beyond to help you investigate the issues you are having, yet my patience is starting to wear thin as well. For starters, you still haven't posted your configuration files, so nobody can even _begin_ to help you with that. As I mentioned in my last post, please post those to your _other_ issue, and let this issue focus on the menu bug.
And just to clarify, I am just a Marlin user, like you. I've contributed a few small changes, and hope to contribute more, but I am in no way officially affiliated with the Marlin project, and do not speak for those who are.
Something is being missed here and that is I am not even working with the menu as I only found the issue when I was trying to figure out what was happening and stumbled upon the issue in the menu. The main issue is that when I am configured with this -> { 80, 80, 4800, 94.06 } Z is dead during a print BUT outside of a print I tell Pronterface to move Z 10mm and my calipers are on Z it moves Z by 10-10.1 but in a print it is dead and disengaged. Now with this -> { 80, 80, 2400, 94.06 } Z is active during a print and works as it should but is off (of course).
I suggest you also post these to your other open issue and continue the _movement_ discussion there. Let this issue focus only on the bug in the _menu_. If the movement issue also turns out to be a bug, it is going to have a different root cause with a different resolution.
@sjasonsmith Done and I hope someone spots something as I posted I have a new issue if I change the microstepping and the steps per mm.
it seems limited to the menu implementation to me
That should be relatively simple to fix. The original tweak reducing the menu edit items to 32768 discreet steps makes some sense, since you seldom want to spin the knob so much. I think the real issue here is that we should never use non-integer steps-per-mm values, so the edit field should be changed to integer-only. Then it will be able to handle 0-32768.
To do that, just change float51
in menu_advanced_steps_per_mm
to float5
. I will apply this patch shortly.
@thinkyhead Thank you for this fix.
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.
Most helpful comment
The particular int16_t issue I found does not go deeper than the LCD menu. I replied to the other issue DarkAlchy opened that I've reconfigured my printer to use 256 microsteps so I could get up to 6400 steps/mm, and it is still working fine with the latest state of 2.0. I also verified I could save large values to the EEPROM and persist them past a power cycle.
I looked around to see if I could find anywhere else that might be casting the value to an int16_t, and it seems that everywhere else handles it as a float or a uint32_t. It is possible there is _something_ I'm missing that doesn't impact my printer, but it seems limited to the menu implementation to me.
Lots of printers in the examples use 4000+ steps/mm on the Z, so I suspect more people would be having the issue if actual movement were broken. I doubt many people configure XYZ steps through the LCD, so few people would be impacted by the menu bug.
**I suggest scoping this as an actual bug, but only for the LCD menu items not allowing floats larger than 3276.8. I hope the details I have provided are sufficient to confirm there is a bug in the menu.
Anything related to actual Z axis movement seems likely to be an unrelated configuration issue, and should be dealt with in DarkAlchy's other issue.**
I'm willing to look into addressing the menu issue, but I won't be able to look at it further for at least a week.