maded2 / plotng

PlotNG - plotting utility for Chia.Net
Apache License 2.0
128 stars 24 forks source link

Tidying up #60

Closed torrayne closed 3 years ago

torrayne commented 3 years ago

This PR is mostly over industry standards for Go code. Feel free to cherry pick.

I used go-lint and tackled as many warnings as I could. The remaining warning are about doc comments. Example: exported type Client should have comment or be unexported

Fix: Replace fmt.Sprintf with log.Printf for plot log file creation error Change: Replace fmt.Printf with log.Printf at internal/server.go:57

Everything else

maded2 commented 3 years ago

I believe ActivePlot needs to be Public struct as otherwise gob don't work properly. Also some of the variable name changes means that current running server will not work with newer version of the UI.

torrayne commented 3 years ago

I didn't find useful information about public vs private structs and gob/reflect but I've re-exported ActivePlot anyway. Could you point me toward a variable name change that breaks newer versions of the UI? I'm not sure I understand.

squizzling commented 3 years ago

ActivePlot is a type that's serialized, and the field names are part of that serialization. As such, they need to be in sync between client and server, or the client (at best) can't see them, or (at worst) de-serializes the entire thing improperly.

maded2 commented 3 years ago

@djatwood looks like something broken with the UI. Please investigate. Thanks.

torrayne commented 3 years ago

Will do

torrayne commented 3 years ago

TLDR: You need to restart the server after updating.

I started plotng using the maded2:main version. Then installed my branch and started the ui. I got the only shows one line in active and archived plots.

Then I restarted the plotng server and ui and everything worked just fine.

The issue is that ActivePlot Id got renamed to ID. So the messages were parsed but ID was empty. This added an empty key to the activeLogs map and every plot was placed here replacing the previous plot.

func (client *Client) drawActivePlotsTable() {
    ...
    for host, msg := range client.msg {
        for _, plot := range msg.Actives {
            // here we have plot.ID but it received plot.Id so plot.ID is empty
            delete(keysToRemove, plot.ID)
            client.activeLogs[plot.ID] = plot.Tail
            client.activePlotsTable.SetRowData(plot.ID, client.makeActivePlotsData(host, plot))
            activePlotsCount++
        }
    }
    ...
}

You could also set explicit JSON tags to prevent this type of issue in the future.

type ActivePlot struct {
    ...
    ID               string       `json:"id"`
    ...
}