Fix duplicate temperature reporting in Octoprint

[Sebazzz]

The duplicate temperature reporting issue is still present on this firmware. We need to fix it.

I got contacted by an individual (to protect his privacy - but still give him credit, I'Il call him RC):

You notice that in Octoprint, sometimes the temperature comes out correct but mostly during a print it comes out wrong. If you issue a M150 command it comes out right. It is only when M155 (auto report) is set that it comes out wrong. Octoprint automatically turns on auto report (see settings->serial connection-intervals & timeouts). You’ll notice that autoreport temperature interval is set to 2s. This is the value used in a M155 command. If set to zero it turns off autoreport and polling automatically begins which uses the M150 command. The temperature comes out correctly when polling. Armed with this information I went digging around in the M150, M155 command code and temperature.cpp code where the temperature is reported. I was looking for a place where it sent the characters twice. The SERIAL_OUT macro defined in serial.h below seemed to be the obvious culprit.

   #define SERIAL_OUT(WHAT, V...) do{ \
      if (!serial_port_index || serial_port_index == SERIAL_BOTH) (void)MYSERIAL0.WHAT(V); \
     if ( serial_port_index) (void)MYSERIAL0.WHAT(V); \
   }while(0)

As you observed, Creality changed MYSERIAL1 to MYSERIAL0. If serial_port_index is set to SERIAL_BOTH the characters (and number strings) will be doubled since both if’s will be true. It just so happens that the only place SERIAL_BOTH is used is in a PORT_REDIRECT in the autoreport code in temperature.cpp (Marlin/src/module) around line 2924 in the file from Creality. What I don’t understand is “where does serial_port_index get changed back after using PORT_REDIRECT”. I’ll have to look at that some more.

   void Temperature::auto_report_temperatures() {
     if (auto_report_temp_interval && ELAPSED(millis(), next_temp_report_ms)) {
       next_temp_report_ms = millis() + 1000UL * auto_report_temp_interval;
       PORT_REDIRECT(SERIAL_BOTH);
       print_heater_states(active_extruder);
       SERIAL_EOL();
     }
   }

“serial_port_index” is typically “1” therefore the second line outputting to MYSERIAL0 is used. It is only when set to SERIAL_BOTH that the characters get output twice. My initial thought was to put MYSERIAL1 back in serial.h and swap the port numbers for SERIAL_PORT and SERIAL_PORT_2. But after looking some more I’m not sure what other side affects this may have. I know this is what is causing the problem, I just don’t know what other things Creality may have done that could be impacted. I hope this gives you something to look at and I’ll look at it again tomorrow to see what has to be done to unravel Creality’s change. I would like to get back as close to Marlin code as possible.

[Sebazzz]

I think Creality changed the macro because serial port 1 MYSERIAL1 is used for communicating with the touch screen:

https://github.com/Sebazzz/Marlin/blob/f3876e81a74b8ffebdef7776e721f7c25e0ce770/Marlin/src/lcd/dwin/LCD_RTS.cpp#L477-L478

https://github.com/Sebazzz/Marlin/blob/f3876e81a74b8ffebdef7776e721f7c25e0ce770/Marlin/src/lcd/dwin/LCD_RTS.cpp#L345-L348

Perhaps we should not let Marlin know about this serial port and initialize it manually.

[NickDevoctomy]

FYI, not sure if this adds anything or not, but there are plugins available for OctoPrint that fix the issue of it not reporting correctly,

Unless this is a different issue of course.

[Sebazzz]

The point is to not need a plugin and undo the changes Creality made to cause this 👍 Met vriendelijke groet, Sebastiaan Dammann

…

________________________________ Van: Nick Pateman <[email protected]> Verzonden: Saturday, September 26, 2020 12:54:50 PM Aan: Sebazzz/Marlin <[email protected]> CC: Sebastiaan Dammann <[email protected]>; Author <[email protected]> Onderwerp: Re: [Sebazzz/Marlin] Fix duplicate temperature reporting in Octoprint (#3) FYI, not sure if this adds anything or not, but there are plugins available for OctoPrint that fix the issue of it not reporting correctly, [image]<https://eur04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fuser-images.githubusercontent.com%2F13888247%2F94339065-0129ba80-ffef-11ea-966c-9f62e5d72e48.png&data=02%7C01%7C%7C7149b3f384bf4159db4908d8620a9841%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637367144919792458&sdata=RYpf1xG0DKGgWeiFmH0KEa2T5KX37eoHKp0No86LvFU%3D&reserved=0> Unless this is a different issue of course. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub<https://eur04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FSebazzz%2FMarlin%2Fissues%2F3%23issuecomment-699479099&data=02%7C01%7C%7C7149b3f384bf4159db4908d8620a9841%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637367144919792458&sdata=C0Ui10MSdGysmTWl6AVRZMHeANg7m7Azcpmn1KcNnnc%3D&reserved=0>, or unsubscribe<https://eur04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAAK4FMISEYO3UC2H6XJHXLDSHXB7VANCNFSM4R2Y4XQQ&data=02%7C01%7C%7C7149b3f384bf4159db4908d8620a9841%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637367144919802453&sdata=HOkGmMVXwYIDAkR7jhL%2FIET5uZBIfSIggp8Qqtld4Lg%3D&reserved=0>.

[NickDevoctomy]

Sure, I was providing an immediate workaround.

[Sebazzz]

I suppose we could just try and revert that macro altogether. I’ve been analyzing the source code for Bugfix-2.0 in combination with the Configuration.h files for the Ender 3 V2 and everything checks out.
The Ender 3 V2 also uses MYSERIAL1 to talk to the touch screen so I think we can simply revert the change in the macro.

[Sebazzz]

Reverted the Creality change in 71ed695. Currently testing.

[Sebazzz]

Fixed in 71ed695

[Sebazzz]

For those who wonder why this works, it appears the touch screen ignores messages it can't handle: