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 118 forks source link

Temperature Graph: filled blue area rather than individual lines #61

Closed SureshotM6 closed 6 years ago

SureshotM6 commented 6 years ago

Screenshots from a USB print: Initial heating, extruder colder that build plate Printing, extruder hotter than build plate

Versions (from About screen): ADVi3++ LCD: 2.1.0 ADVi3++ Mainboard: 2.1.0 LCD Firmware: 4.2 Marlin: 1.1.8

andrivet commented 6 years ago

Apparently, this behavior occurs only with some versions of the LCD firmware (the firmware inside the LCD screen and made by DWIN, not the set of LCD screens). You have version 4.2 and I was only able to test with version 2.0 and 2.2. So will difficult to fix the issue.

SureshotM6 commented 6 years ago

Interesting. I'm getting a development environment setup now to take a look at this myself then. My unit is fairly new and I'm sure that other buying new Di3+'s or Maker Select Pluses will end up with the same issue.

I took a second look at my LCD board as well, and it seems that it is a DMT48270M04302WT instead of the DMT48270M04305W that you mention in your teardown. From what I can tell looking at the datasheets, the differences (besides firmware version) are:

I see that in commit "d1a7ded4e Complete rework for version 2 (smaller functions, less actions, new IDs of Pages, etc.)", you made a change to utilize 4 curves for temperature rather than just 2. And I see from the resources I loaded into the DGUS SDK that you have shifted "Y_Central" by 1 on two of the Curve Displays, presumably to get a wider line.

I'll see what I can do to maintain compatibility with both. I can think of a few options that may work:

andrivet commented 6 years ago

Very interesting and complete information. Thank you. I will try to implement you solution #3. I will see also if I can buy a DMT48270M043_02WT display.

SureshotM6 commented 6 years ago

I think a different screen in the LCD resources is going to be required for the _02WT that contains only 2 curve channels. Here are the results with the current screen that contains 4 channels, but only sending data for 2:

Channels Result
0,1,2,3 blue filled area
0,1 thick blue curve and thin red curve
0,2 red filled area
0,3 red filled area
1,2 blue filled area
1,3 blue filled area
2,3 blank

So the only combination that (somewhat) worked was when using channels 0 and 1. However, both of those channels should have been red extruder temperature curves, so undefined behavior is being exploited for sure. Additionally, the blue curve was definitely 2 pixels tall, so one of the additional channels was being utilized somehow.

andrivet commented 6 years ago

I will not rely on undefined behavior.

SureshotM6 commented 6 years ago

I will not rely on undefined behavior.

It was not my intent to suggest that you do.

I was merely running these tests to determine what exactly is meant by the statement "Only 2 curves are supported in dynamic curve function" in the datasheet. My hope was that this meant that the 0x84 / Trend Curve Buffer command would work properly with the existing screen in the LCD resources, provided that only 2 channels were used in the Trend Curve Buffer command. If it had, it would have made supporting both LCD models easier (provided there is a way of programmatically determining which model is connected). That statement did not necessarily impose additional restrictions such as "only channels 0 and 1 may be used", although it appears this may be the case from these test results.

Given that none of the tests I ran produced a valid result (at least what I was expecting to be valid from the documentation that is available), I believe that I've confirmed that the LCD resources will need to be modified as well to support this, which is all I was saying above. I believe, either both models need to use only 2 curves in both the LCD resources screen and the Trend Curve Buffer command, a duplicate screen needs to be created with only 2 curves to be used for the _02WT models only, or the LCD firmware needs two builds.

I will attempt to regenerate the Temperature Graph screen with only 2 Curve Displays instead of 4, and ensure that properly fixes the issue on the _02WT model.

As for dynamically determining the LCD model, it seems that R0 is not useful from the conversation on issue 63. There are other differences listed in the datasheets as well, so perhaps one of those differences may be queryable.

By the way, thank you for creating this firmware. It is already much better than the stock firmware.

andrivet commented 6 years ago

I will do some test when I receive my 02WT LCD display. But for the moment (i.e. for version 3.0) I will only use 2 channels so it will work with all displays.

SureshotM6 commented 6 years ago

Verified to work as expected with only 2 Curve Displays as channels 0-1 in the LCD Resources screen rather than 4.

I don't see any update for 2.1.0 in the ADVi3pp-LCD repository so I cannot submit a pull request. The change is simple enough though.

andrivet commented 6 years ago

Implemented in version 3.0