Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

regression in websocket init_message caused by commit a912ce4 #509

Open
richpletcher opened this issue May 6, 2024 · 1 comment
Open

Comments

@richpletcher
Copy link

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 added a commit that referenced this issue May 9, 2024
.

Added NGC parameter 5599, debug output enabled status.
@terjeio
Copy link
Contributor

terjeio commented May 9, 2024

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants