grblHAL / core

grblHAL core code and master Wiki
Other
304 stars 73 forks source link

regression in websocket init_message caused by commit a912ce4 #509

Open richpletcher opened 2 months ago

richpletcher commented 2 months ago

The old code in stream.c stream_select:

    case StreamType_WebSocket:
        if(connection_is_up(&hal.stream))
            report_message("WEBSOCKET STREAM ACTIVE", Message_Plain);
        if(add && sys.driver_started && !hal.stream.state.webui_connected) {
            hal.stream.write_all = stream->write;
            grbl.report.init_message();
        }
        break;

Temporarily switched hal.stream.write_all to be the new stream's write and called report.init_message which calls hal.stream.write_all and thus sent the init_message to the new stream only. (This code is repeated for the other stream types.)

The new code: case StreamType_WebSocket: if(connection_is_up(&hal.stream)) report_message("WEBSOCKET STREAM ACTIVE", Message_Plain); if((send_init_message = add && sys.driver_started && !hal.stream.state.webui_connected)) hal.stream.write_all = stream->write;

        break;

Temporarily switches hal.stream.write_all for no reason because it is immediately replaced with following memcpy. But later the new code has:

if(send_init_message)
    grbl.report.init_message();

This is fine, but the standard write_all code loops thru connections and calls the connection's stream->write for all "up" connections, however; in networking plugin websocketd.c function websocket_claim_stream calls stream_connect (which calls stream_select) before setting "ws_streams[0].flags.connected = true". So, the result is the init message goes to the original serial stream, but not to the websocket stream since it doesn't appear to the "up" yet.

The fix is either to back out the changes to stream, or to change websocket_claim_stream to set flags.connected before calling stream_connect. The 2nd option would result in the init message being broadcast to all connections which is still a change in behavior.

terjeio commented 1 month ago

I have changed the code again, not reverted the previous change since that fixed another issue with native USB streams. I have also modified stream switching depending on a MPG beeing in control when a network stream is connected/disconnected. Hopefully this is ok - I have been very busy lately so have not got enough time for testing...

Anyway, thanks for looking into this and reporting the issue!