FlowFuse
Not blocked "Undefined" in ui-chart legend
Current Behavior
When sending any lines in the data without the necessary keys "series" and "value" , ui-chart works correctly, but returns "undefined" in the legend.
Expected Behavior
Need ability to block "undefined" in legend
Steps To Reproduce
Make the parameter "block undefined in legend" in the ui-chart settings
Environment
- Dashboard version:
- Node-RED version:
- Node.js version:
- npm version:
- Platform/OS:
- Browser:
Have you provided an initial effort estimate for this issue?
I am not a FlowFuse team member
You can hide the legend already in the chart settings
I need hide only undefined, not all legend.
Right, but the only use case where this occurs is when you have a single line of data you're passing to the chart, so there is no need for a Legend? Or, change the "Series" to a hard-coded string to give the line a label.
It is undefined, because the label for the line is quite literally undefined
Right, but the only use case where this occurs is when you have a single line of data you're passing to the chart, so there is no need for a Legend? Or, change the "Series" to a hard-coded string to give the line a label.
It is
undefined, because the label for the line is quite literally undefined
I have a single data flow for many diagrams and some rows for one, some rows for another one. The diagrams themselves now read their own data and do not draw other data. But "undefined" is added to such other rows in the legend. I have normal legend + "undefined". I need block this.
Example: var output = [ { 'series400': "Common voltage", 'values400': 401 }, { 'series230': "Voltage A", 'values230': 235 }, { 'series230': "Voltage B", 'values230': 230 }, { 'series230': "Voltage C", 'values230': 232 }, { 'series400': "Voltage A", 'values400': 402 }, { 'series400': "Voltage B", 'values400': 400 }, { 'series400': "Voltage C", 'values400': 401 } ];
msg.payload = output; return msg;
Ah, okay, so you have instances where a datapoint passed to your chart doesn't actually contain any data to plot. The chart is trying, but failing to, and therefore adding an undefined series?
If so, that makes sense, thank you for clarifying, and something we should handle.
Yes, you understand everything absolutely correctly now. Exactly like that.
If you're a VueJS developer, you're welcome to dive in and try to fix it. Otherwise, it'll be a matter of waiting until someone is able to pick it up.
If you're a VueJS developer, you're welcome to dive in and try to fix it. Otherwise, it'll be a matter of waiting until someone is able to pick it up.
I am not a VueJS developer, I can't even imagine what this VueJS is. But I corrected the already compiled file and now everything works correctly. It is necessary to correct the text: add (msg) { const payload = msg.payload // determine what type of msg we have if (Array.isArray(msg) && msg.length > 0) { // we have received an array of messages (loading from stored history) msg.forEach((m, i) => { const p = m.payload const d = m._datapoint // server-side we compute a chart friendly format const label = d.category this.addPoints(p, d, label) }) } else if (Array.isArray(payload) && msg.payload.length > 0) { // we have received a message with an array of data points // and should append each of them payload.forEach((p, i) => { const d = msg._datapoint ? msg._datapoint[i] : null // server-side we compute a chart friendly format where required const label = d.category this.addPoints(p, d, label) }) } else if (payload !== null && payload !== undefined) { // we have a single payload value and should append it to the chart const d = msg._datapoint // server-side we compute a chart friendly format const label = d.category this.addPoints(msg.payload, d, label)
to: add (msg) { const payload = msg.payload // determine what type of msg we have if (Array.isArray(msg) && msg.length > 0) { // we have received an array of messages (loading from stored history) msg.forEach((m, i) => { const p = m.payload const d = m._datapoint // server-side we compute a chart friendly format const label = d.category if (label !== null && label !== undefined) { this.addPoints(p, d, label) } }) } else if (Array.isArray(payload) && msg.payload.length > 0) { // we have received a message with an array of data points // and should append each of them payload.forEach((p, i) => { const d = msg._datapoint ? msg._datapoint[i] : null // server-side we compute a chart friendly format where required const label = d.category if (label !== null && label !== undefined) { this.addPoints(p, d, label) } }) } else if (payload !== null && payload !== undefined) { // we have a single payload value and should append it to the chart const d = msg._datapoint // server-side we compute a chart friendly format const label = d.category if (label !== null && label !== undefined) { this.addPoints(msg.payload, d, label) }
in the file: https://github.com/FlowFuse/node-red-dashboard/blob/main/ui/src/widgets/ui-chart/UIChart.vue
I.e. You need to add if (label !== null && label !== undefined)
I am not a VueJS developer, I can't even imagine what this VueJS is.
But at least you tried to find a solution, and it seems you even managed to fix it. Kudos!! Such kind of help is very welcome here, because you pinpointed the location in the code where things go wrong.
By looking at your fix, it "looks" to me that you found an issue that is not only related to the legend.
In your example you have datapoints that don't contain the key property, which has been specified in the config screen.
While your fixed version is very simple and neat, these datapoints (without the key property) are still being pushed to the frontend, where you skip them.
But in fact it is quite a big overhead that the chart node stores all these faulty datapoints in its memory, and pushes all these datapoints to the frontend where they will be skipped. Moreover after a refresh of the browser page, I assume all these datapoints will be replayed and ignored again. At the end you managed to keep them away from your dashboard ui, but under the cover they will still be used all over the place. That probably won''t be a problem for your use case, but it will be quite a resource bottleneck when a lot of such faulty datapoints are being injected...
I have not tested this, but based on your fix code I "think" you can drop all these faulty datapoints as soon as they are being injected into the ui-chart node in the server-side code. Something like this I think, where I have added my comments in capitals, of code I "think" needs to be changed.:
let series = RED.util.evaluateNodeProperty(config.category, config.categoryType, node, msg)
// if receiving a object payload, the series could be a within the payload
if (config.categoryType === 'property') {
series = getProperty(p, config.category)
}
// single point or array of data?
if (Array.isArray(p)) {
// array of data
msg._datapoint = p.map((point) => {
// series available on a msg by msg basis - ensure we check for each msg
if (config.categoryType === 'property') {
series = getProperty(point, config.category)
}
// IF THE KEY IS NOT AVAILABLE THEN SKIP THIS DATAPOINT
if (!series) {
return null
}
return addToChart(point, series)
}).filter((point) => point !== null) // REMOVE THE NULL VALUES FROM THE LIST, WHICH YOU HAVE RETURNED ABOVE
} else {
// single point
if (config.categoryType === 'json') {
// we can produce multiple datapoints from a single object/value here
const points = []
// MAKE SURE THE SERIES ARE NOT UNDEFINED
if (series) {
series.forEach((s) => {
if (s in p) {
const datapoint = addToChart(p, s)
points.push(datapoint)
}
})
}
msg._datapoint = points
} else {
// MAKE SURE THE SERIES ARE NOT UNDEFINED
if (series) {
msg._datapoint = addToChart(p, series)
}
}
}
However by doing it this way, the msg._datapoint might now become undefined or [] by skipping the datapoints without the key property. That means we have to be aware of that on the frontend-side code also, to make sure there is no crash:
add (msg) {
const payload = msg.payload
// determine what type of msg we have
if (Array.isArray(msg) && msg.length > 0) {
// we have received an array of messages (loading from stored history)
msg.forEach((m, i) => {
// SKIP THE DATAPOINT IF NOT AVAILABLE
if (m._datapoint) {
const p = m.payload
const d = m._datapoint // server-side we compute a chart friendly format
const label = d.category
this.addPoints(p, d, label)
}
})
} else if (Array.isArray(payload) && msg.payload.length > 0) {
// we have received a message with an array of data points
// and should append each of them
// SKIP THE DATAPOINTS IF NOT AVAILABLE
if (msg._datapoint && Array.isArray(msg._datapoint) && payload.length === msg._datapoint.length) {
payload.forEach((p, i) => {
// CODE SIMPLIFIED BECAUSE WE KNOW THAT THE DATAPOINT EXISTS HERE
const d = msg._datapoint[i] // server-side we compute a chart friendly format where required
const label = d.category
this.addPoints(p, d, label)
})
}
} else if (payload !== null && payload !== undefined) {
// SKIP THE DATAPOINT IF NOT AVAILABLE
if (msg._datapoint) {
// we have a single payload value and should append it to the chart
const d = msg._datapoint // server-side we compute a chart friendly format
const label = d.category
this.addPoints(msg.payload, d, label)
}
} else {
// no payload
console.log('have no payload')
}
if (this.chartType === 'line' || this.chartType === 'scatter') {
this.limitDataSize()
}
this.update()
},
Don't hesitate to aks me if something is not clear. I have not tested this code, due to lack of free time!!
Bart
Don't hesitate to aks me if something is not clear. I have not tested this code, due to lack of free time!!
if you delete on the server side (I did it at beginning), then the output of the diagram will not get the original flow, but a truncated one and the next diagram or next some other node will not receive the necessary data. you definitely don't need to do this!!
I sent the correct code that does not spoil the flow
Ah yes, good point.
As mentioned above I have not yested my code. But at first sight I wouldn't expect my code to change the original input message. So if you enable passthrough, I would expect the next nodes in your flow to get all the original data.
Please correct me if I am wrong. I don't know the internals completely for this chart node, so perhaps the modified data somehow finds its way to the output...
Ah yes, good point.
As mentioned above I have not yested my code. But at first sight I wouldn't expect my code to change the original input message. So if you enable passthrough, I would expect the next nodes in your flow to get all the original data.
Please correct me if I am wrong. I don't know the internals completely for this chart node, so perhaps the modified data somehow finds its way to the output...
How can I make sure that at least some kind of fix is taken in release ?
You will need to get it into a pull request, so the Flowfuse people can review and merge it into the master branch of this repository. Take into account that they are currently very busy (on other non-dashboard related stuff) so not sure when they will get to it.
Meanwhile - since you have already experimented to filter the datapoints on the server side - it would be very useful if you could also post here how you did that, and why it didn't work. As described in my last post, I don't really understand at the moment why that wouldn't work. Your feedback would really be helpful for any contributor that wants to implement server-side filtering in the future. Thanks!!
You will need to get it into a pull request, so the Flowfuse people can review and merge it into the master branch of this repository. Take into account that they are currently very busy (on other non-dashboard related stuff) so not sure when they will get to it.
Meanwhile - since you have already experimented to filter the datapoints on the server side - it would be very useful if you could also post here how you did that, and why it didn't work. As described in my last post, I don't really understand at the moment why that wouldn't work. Your feedback would really be helpful for any contributor that wants to implement server-side filtering in the future. Thanks!!
Unfortunately I can't push, I was asked to help, find a place and fix it, I found it, but maybe your version is better. When I made empty datapoint strings on the server side, I got an error that somewhere they were accessing a property of a non-existent object. This error was not on the client, but on the server and was output to the node-red console. Component did not work and did not have output flow.
When I made datapoints smaller, only in terms of data quantity, everything in the component moved apart. Then I had to delete the lines from msg and not write to datapoints at the same time. It's work, but the output flow was corrupted
In theory, your version with empty datapoints should cause an error somewhere on the server side.
Thanks for your explanation! That will be helpful if anybody wants to look at this in more depth in the near future.
Unfortunately I can't push
Do you mean you need help to create a pull request for your workaround? If yes, I can explain the basic steps.
Do you mean you need help to create a pull request for your workaround? If yes, I can explain the basic steps.
If you can make this request, that would be great. It would be impossible for me to do it from my phone.
I don't claim any copyright.
@teluse7 Here you go.
I don't claim any copyright.
First I thought: lol, that is a good one. But indeed I had signed an agreement at the time being, that I don't claim any copyright. So good from you to mention it!!
Now you have to wait until Joe has time to review and merge it, but he is very busy at the moment with other stuff unfortunately...
I don't claim any copyright.
First I thought: lol, that is a good one. But indeed I had signed an agreement at the time being, that I don't claim any copyright. So good from you to mention it!!
It wasn't about copyright in the legal sense. I just said that I don't know how to push and most likely I won't be able to technically. But if you can do it, that would be great. I don't claim that it will be written that I found a bug. If you push and it will be considered that you found a bug, that's also great. That's what I'm talking about. I helped as much as I could.
Now you have to wait until Joe has time to review and merge it, but he is very busy at the moment with other stuff unfortunately...
Hello. I saw that this change was added somewhere, but there was an error in the tests. It seems that the tests concern something else. Need to find another error?
@teluse7 Very often the unit tests fail due to something unrelated, i.e. in tests of another ui widget. This time however one of the failing tests is inside the ui-chart node itself:
So I first thought that your fix perhaps somehow caused that test to fail.
That failing unit test seems to use this test flow.
When I import that test flow in my Node-RED manually and I test that flow I indeed get a chart containing only a single point:
But the failing unit test is not caused by your fix, because I have run it on a Node-RED instance that doesn't contain your fix. So no idea why the test fails. I don't have enough free time to figure that all out on my own...
Hello I fix problem more than half year ago.
I was told that we need people who could fix such errors, because we do not comply. But in the end, no one has included anything in the version for more than half a year. Moreover, the test in some other place did not pass and this error continues to live. The new version is compiled with it, but my fix cannot be compiled because of it. What is happening?
If you're a VueJS developer, you're welcome to dive in and try to fix it. Otherwise, it'll be a matter of waiting until someone is able to pick it up.
@joepavitt half a year has passed since the correction and no one needs anything?
Thanks for flagging @teluse7 - I've assigned myself as a reviewer, and will take a look. Not sure how that's consistently slipped past the net.
Thanks for flagging @teluse7 - I've assigned myself as a reviewer, and will take a look. Not sure how that's consistently slipped past the net.
@joepavitt the error identified by the failed test existed before my edit and still exists in the new versions, but for some reason this error doesn't prevent the release of new versions, but it does prevent my fix from being applied. This is strange. I also noticed that additional code was added over time, and it was not wrapped in an if statement to check for emptiness.
There are 3 x errors in the tests. The ui-control test failure has been around for a long time, and is inconsistent. There is a new failure within the tests associated to the chart though, which is a new problem and needs investigation.