search

g4klx / MMDVMHost

The host program for the MMDVM
GNU General Public License v2.0
369 stars 268 forks source link

Nextion Screen Commands no longer recieved by NextionDriver #796

Open MW0MWZ opened 4 months ago

MW0MWZ commented 4 months ago

Commit #a9fbbc1 (24-Oct-2021) works, and the commands from the screen to NextionDriver are working. Commit #fc836f3 (24-Oct-2021) stops working, and from here on it never works again.

Data from MMDVMHost / NextionDriver out to the screen works just fine.

EDIT: Improved commit links

g4klx commented 4 months ago

This doesn't appear to make a lot of sense. However I've made a change that partially rolls back the breaking commit and I hope that helps.

MW0MWZ commented 4 months ago

As you might already expect, that made no difference, given the scope of that commit (not a C++ programmer) but I also agree that I would have been surprised if that cured it. Let me verify the before/after that commit and see if I can use diff and check if there are any other changes, just in case they were in a merged branch or something.

MW0MWZ commented 4 months ago

Firstly, sorry - not sure how my testing lead me to those two commits, but they are certainly not it, the last change you made, you can probably revert back to where it was (or leave it - as you think is best).

So far as I can tell, there are two issues in play here, something changes between [#12282f7] (https://github.com/g4klx/MMDVMHost/commit/12282f709a6512be7d8667d2610760b666e90ad2) (Working at this point, with MMDVM_HS 1.5.2 from CA6JAU), and #c469d31 (stops working with MMDVM_HS 1.5.2 from CA6JAU).

However this becomes more awkward when I also try to upgrade the FW on the hat to 1.6.1 (from your MMDVM_HS repo), neither version work with that.

Looking at the commit that stops working, it looks un-related, but when I checkout the two commits into separate folders and check the changes between the two, they are very significant. I can pull both and submit both back to github to create a commit to show all the changes if that help, let me know if that would be useful.

All testing done using the Makefile.Pi.OLED makefile, if there is anything else I can do, or any other information I can provide, please let me know.

g4klx commented 4 months ago

I can confirm that v1.6.1 of the HS firmware is very well tested indeed, so I do not think that the problem lies there. The Gid Ids also make no sense, and so a more thorough diff is needed since the changes I see are to do with the HD44780 display which is irrelevant to the Nextion driver.

I need considerably more information before I can take any action. I have got no similar reports of issues from users of WPSD but I will act on them if they are reported of course.

MW0MWZ commented 4 months ago

The actual change between those two points in the commit history look like this: https://github.com/MW0MWZ/MMDVMHost/commit/2de7cdc5ef3c9fe126f788dc318dcc10db14c3d4

A LOT gets rolled in at that point. I am getting reports that users see the same thing in WPSD, but I agree more information and more reports help you see what is going on.

The firmware (1.6.1) works perfectly in every other regard I can find, as does MMDVMHost - the only issue I have seen so far, is the interaction coming back from the Nextion screen, that would normally show up in NextionDriver.

I'll leave this open for now and see if anyone else adds to it, if not feel free to close it in a few days time.

W0CHP commented 4 months ago

Issue confirmed :-(

WPSD currently ships with 06a3da0 (2024-02-07).

Reboot/Shutdown "buttons" from a 3.5" enhanced NX display do nothing, and NextionDriver doesn't report anything in the syslog. This is on an MMDVM_HS v1.6.1 instance.

Checking out and compiling commit 12282f7 as Andy referenced, and leaving the firmware at v1.6.1, commands from the "buttons" on the display work as intended and perform the requested commands/actions. From the syslog:

Feb 15 13:25:48 wpsd-nx-tst NextionDriver: Received command 0xF1 
Feb 15 13:25:48 wpsd-nx-tst NextionDriver:  Execute command "reboot"

Checking out & compiling commit c469d31 as Andy also referenced, exhibits the non-working touch action/command behavior; with no NextionDriver messages in the syslog.

I'm very surprised this wasn't logged as an issue in the WPSD mediums, honestly. :-O

I can help poke around in the differences between commits a bit more tomorrow in the attempt to help squash this thing.

MW0MWZ commented 4 months ago

The change between the two commits, is 12,000+ lines long - more eyes is more eyes, and we all benefit. Not sure why nobody reported it back to you yet :( but I am sure you would have heard about it at some point. Anything either of you need from me to help with this - please ask, like I said we all benefit from improvements.

W0CHP commented 4 months ago

The change between the two commits, is 12,000+ lines long - more eyes is more eyes, and we all benefit. Not sure why nobody reported it back to you yet :( but I am sure you would have heard about it at some point. Anything either of you need from me to help with this - please ask, like I said we all benefit from improvements.

Wholeheartedly agree. Once I get the kids off to school tomm., I'll have the bandwidth (and mental capacity lol) to parse through the changes. Thanks!

W0CHP commented 4 months ago

My dogs awoke me at 2am and I could not go back to sleep. In my groggy state, I found that between the two referenced commits, this was changed in Modem.cpp:

Present in working commit:

            case MMDVM_SERIAL:
                //MMDVMHost does not process serial data from the display,
                // so we send it to the transparent port if sendFrameType==1
                if (m_sendTransparentDataFrameType > 0U) {
                    if (m_trace)
                        CUtils::dump(1U, "RX Serial Data", m_buffer, m_length);

                    unsigned char offset = m_sendTransparentDataFrameType;
                    if (offset > 1U) offset = 1U;
                    unsigned char data = m_length - 3U + offset;
                    m_rxTransparentData.addData(&data, 1U);

                    m_rxTransparentData.addData(m_buffer + 3U - offset, m_length - 3U + offset);
                    break; //only break when sendFrameType>0, else message is unknown
                }
            default:
                LogMessage("Unknown message, type: %02X", m_buffer[2U]);
                CUtils::dump("Buffer dump", m_buffer, m_length);
                break;

Changed in non-working commit:

            case MMDVM_SERIAL_DATA:
                if (m_trace)
                    CUtils::dump(1U, "RX Serial Data", m_buffer, m_length);
                m_rxSerialData.addData(m_buffer + m_offset, m_length - m_offset);
                break;

            default:
                LogMessage("Unknown message, type: %02X", m_type);
                CUtils::dump("Buffer dump", m_buffer, m_length);
                break;

I am unsure if that (the removal of the Transparent Data stuff) is the culprit...I'm very tired, so it's entirely possible I am way off here.

g4klx commented 4 months ago

The transparent data handling hasn't changed. I renamed MMDVM_SERIAL to MMDVM_TRANSPARENT with the same code and a more accurate debug statement, MMDVM_SERIAL_DATA is something else.

What I don't get, is that the Nextion driver in the MMDVM Host is a write-only driver, it doesn't handle incoming commands from a touch screen, and never has. So all commands are handled outside of the MMDVM Host and I would humbly suggest are not a problem with the MMDVM code, at least not without a LOT more evidence.

W0CHP commented 4 months ago

@g4klx makes sense. I am not 100% clear on how commands from the display are handled, honestly. I’ll keep digging. I agree; I’d rather have more info as to why X works on commit Y, but not on commit Z. I’m trying! ;-)

on7lds commented 4 months ago

MMDVMHost does not process incoming serial data from the Nextions, one of the reasons to use NextionDriver.

If NextionDriver is installed and configured, it is not MMDVMHost who is talking to the display, but NextionDriver. I agree with @g4klx, I also can not see an obvious reason why a MMDVMHost update should stop NextionDriver from receiving display data.

I have a Pi-Star running Pi-Star:4.1.6 / Dashboard: 20240218, MMDVMHost version 20240210_PS4 git #218a017, DVMEGA HR3.14, NextionDriver version 1.25 No problem with the screen buttons.

If NextionDriver was processing screen buttons and stopped to do so (while no configuration changes are made), the only thing I can think of at this moment is another process reading the serial data from the serial port before NextionDriver does.

update: Unless it's a modem-connected display, ofcourse. Then the data pass via MMDVMHost. But they should just pass...

MW0MWZ commented 4 months ago

@ON7LDS just to be clear here, are you using a USBTTL device to drive the screen (this works perfectly still on the newest builds) or are you driving it from the port on the MMDVM_HS hat? (I would assume since you are using a DVMEGA device that you are using a USB TTL).

I agree that MMDVMHost is not doing any processing of the data, but screen interaction was working previously, and now it is not longer seen by NextionDriver when using the modem port.

on7lds commented 4 months ago

Using a USBTTL, indeed. I jus saw indeed it is more of an modem connected issue. I'll try to make a setup like that to (hopefully) replicate the problem.

on7lds commented 4 months ago

I had some time to take a first look. Using _MMDVMHost version 20240210PS4 git #218a017 and _MMDVMHost version 20210617PS4 git #9106fd6

I used 'Trace=1" and 'Debug=1' in the [modem] section. Both binaries generate debug lines like

D: 2024-02-21 15:21:16.196 RX Serial Data
D: 2024-02-21 15:21:16.196 0000:  E0 09 80 2A FB 04 FF FF FF                         *...*.....*

which is correct.

However, tcpdump shows that no packets are sent out with the newer binary, and they are sent out by the old one. So it seems that somewhere in MMDVMHost the serial data is not processed further or the UDP packets are not sent out for some reason (I hav no idea of the cause. I have to leave in a while; I probably cannot search further today)

g4klx commented 4 months ago

It must have been a horrible bit of code that I removed then! I was tidying it all up a few years ago and this crept through. This change happened three years ago, I am deeply unhappy that it has taken three years for this to be found, in fact it's bloody disgusting. I don't expect to have to return to major bugs so long after the fact. The good news is that the whole display code architecture is changing radically and your display driver can be rolled into the my basic output only display driver in a much nicer manner than now. The changes will be worth it. Jonathan  G4KLX

On Wednesday, 21 February 2024 at 15:39:36 GMT, on7lds ***@***.***> wrote:

I had some time to take a first look. Using MMDVMHost version 20240210_PS4 git #218a017 and MMDVMHost version 20210617_PS4 git #9106fd6

I used 'Trace=1" and 'Debug=1' in the [modem] section. Both binaries generate debug lines like D: 2024-02-21 15:21:16.196 RX Serial Data D: 2024-02-21 15:21:16.196 0000: E0 09 80 2A FB 04 FF FF FF ........*

which is correct.

However, tcpdump shows that no packets are sent out with the newer binary, and they are sent out by the old one. So it seems that somewhere in MMDVMHost the serial data is not processed further or the UDP packets are not sent out for some reason (I hav no idea of the cause. I have to leave in a while; I probably cannot search further today)

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: @.***>

g4klx commented 4 months ago

I have added a new back channel for the Nextion to send data out of a UDP port. There are two new configuration options in the Nextion configuration stanza which enables the back channel NextionOutput=0|1" and "NextionUDPPort=NNNN" which is the port to send the data to. The source and destination IP addresses are fixed as 127.0.0.1 and the host will use a port number that is one less than the port number given in NextionUDPPort.

DL9AM commented 4 months ago

Hello Jonathan

very good and fast work ;-)

so must on7lds make a change for Port on NextionDriver ? And then mw0mwz and w0chp a update with new Binar from your new MMDVMHost and new NextionDriver ?

And must copy the new 2 last entry from MMDVM.ini to mmdvmhost...

Best regard from Germany 73 DL9AM Marco

on7lds commented 4 months ago

@g4klx : sadly this breaks one thing to solve another. The working configuration with the 2021 code was

[Nextion]
Port=/dev/ttyNextionDriver

[NextionDriver]
Port=modem

This means MMDVMHost will pass output to NextionDriver via the virtual serial port, where it is processed and passed on to the display. The NextionDriver configuration (Port=modem) tells NextionDriver 1) to pass the data for the display via UDP to MMDVMHost 2) to listen on UDP to get transparent data from MMDVMHost originating from the display (button clicks).

The current solution with the backchannel is :

But this way, NextionDriver can only process the display button data when not allowed to process the data to the display.

g4klx commented 4 months ago

Please look at the new version and in particular the way the configuration is different compared to the old version. The old method of working was s**t and the new display framework will be considerably better. These new changes are designed to enable your driver to work until the new framework is fully proven.

Sent from Yahoo Mail for iPhone

On Sunday, February 25, 2024, 16:08, on7lds @.***> wrote:

@g4klx : sadly this breaks one thing to solve another. The working configuration with the 2021 code was [Nextion] Port=/dev/ttyNextionDriver

[NextionDriver] Port=modem

This means MMDVMHost will pass output to NextionDriver via the virtual serial port, where it is processed and passed on to the display. The NextionDriver configuration (Port=modem) tells NextionDriver 1) to pass the data for the display via UDP to MMDVMHost 2) to listen on UDP to get transparent data from MMDVMHost originating from the display (button clicks).

The current solution with the backchannel is :

But this way, NextionDriver can only process the display button data when not allowed to process the data to the display.

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: @.***>

shidai-dev commented 3 months ago

@g4klx : sadly this breaks one thing to solve another. The working configuration with the 2021 code was

[Nextion]
Port=/dev/ttyNextionDriver

[NextionDriver]
Port=modem

This means MMDVMHost will pass output to NextionDriver via the virtual serial port, where it is processed and passed on to the display. The NextionDriver configuration (Port=modem) tells NextionDriver 1) to pass the data for the display via UDP to MMDVMHost 2) to listen on UDP to get transparent data from MMDVMHost originating from the display (button clicks).

The current solution with the backchannel is :

  • MMDVMHost talks to the display and sends UDP packets for data from the display (Port=modem) OR
  • MMDVMHost talks to a serial port (Port=/dev/tty) and has no knowledge of data from a modem connected display

But this way, NextionDriver can only process the display button data when not allowed to process the data to the display.

Yes, I tried to add code to Modem.cpp to make the MMDVMHost process pass serial data to the Nextion driver via UDP, although the screen touch worked fine, the screen could not receive data to the screen. I know there might be an error in the code, I'm just trying to verify that, passing m_rxSerialData through the transparentSocket, the reality is that the screen button does work

[MMDVMHost.cpp]

len = m_modem->readSerial()
transparentSocket->write(data,len,transparentAddress,transparentAddrLen);

[Modem.cpp] 
case MMDVM_SERIAL_DATA:
 m_rxSerialData.addData(m_buffer + m_offset - offset, m_length - m_offset + offset);
int CModem::readSerial()
 m_rxSerialData.getData(data,len);

Starting in console mode...
NextionDriver version 1.25
Copyright (C) 2017...2021 ON7LDS. All rights reserved.
Starting with verbose level 2
Reading configuration file /etc/mmdvmhost
Found RX Frequency 433750000
Found TX Frequency 439750000
Found Location [Town, L0C4T0R]
Use Transparent Connection: YES
  Local port: 40094
  Remote port: 40095
  Send Frame Type: YES
Found Virtual Port [/dev/ttyNextionDriver]
Found Nextion Port [modem]
RemoveDim: OFF
WaitForLan: ON
SleepWhenInactive: OFF
Groups file will be fetched from [https://hostfiles.w0chp.net/groupsNextion.txt]
Using verbose level 2
Running on Raspbian GNU/Linux 11 (bullseye)
Pi-Star v 4.1.6
Opening ports
 /dev/ttyNextionDriver (=/dev/pts/2) <=> modem
Data files directory: /usr/local/etc/
 Groups file : 2024-03-29 18:24:25 (23 hour old)
 Users file : 2024-03-30 17:22:42 (17 min old)
  Reading groups from /usr/local/etc/groupsNextion.txt
  Read 2 groups.
  Reading users from /usr/local/etc/stripped.csv
   delimiter ','
   DMRid in field 1
   Call  in field 2
   Name  in fields 3 + 4
   Extra data in fields 5,6 and 7
  Read 145873 users in 1336 ms.
  Sorted CALL table in 503 ms.
Disk size : 29784 MB (26668 free)
Started with screenLayout 3
Started with verbose level 2
Opening sockets ...
Transparent Connection: talking socket open, fd=4
Try to bind 0.0.0.0 ...
Transparent Connection: listening socket open, fd=5
Transparent data sockets active
I can not (yet) check or update modem connected displays
Starting with network interface wlan0:192.168.31.122
Received command 0xFB/4
Sending OS info

Received command 0xFB/2
 Groups file : 2024-03-29 18:24:25 (23 hour old)
 Users file : 2024-03-30 17:22:42 (17 min old)
Received command 0xF1
 Execute command "sudo HostFilesUpdate.sh"
 Command response "W0CHP Hostfile Update Server connection OK...updating hostfiles."