parallaxinc / solo

BlocklyProp without the wires
MIT License
5 stars 5 forks source link

ColorRemap block is generating NaN in first argument #434

Closed zfi closed 4 years ago

zfi commented 4 years ago

The simple project below is generating code with a 0x NaN in the remapColor() call.

Screen Shot 2020-05-15 at 5 41 34 PM
#include "simpletools.h"
#include "ssd1331.h"
#include "colormath.h"

// ------ Global Variables and Objects ------
screen *oledc;// ------ Libraries and Definitions ------

// ------ Main Program ------
int main() {
  oledc = ssd1331_init(17, 6, 5, 3, 2, 96, 64);
  fillRect(oledc, 0, 0, 96, 63, remapColor(0x NaN, "8R8G8B", "5R6G5B"));
}
zfi commented 4 years ago

The C code generator for the color_value_from block is returning a function call.

getColorRRGGBB(50, 50, 50)

The C code generator for the OLED triangle block is expecting a hexadecimal value in RRGGBB format. The actual call to getColor() does return an appropriate value, so the call should be inlined in the C code output and not converted to a hex value.

In this example project, using a color value generates reasonable code.

Screen Shot 2020-05-21 at 5 50 05 PM
// ------ Libraries and Definitions ------

#include "simpletools.h"
#include "ssd1331.h"
#include "colormath.h"

// ------ Global Variables and Objects ------
screen *oledc;

// ------ Main Program ------
int main() {
  oledc = ssd1331_init(17, 6, 5, 3, 2, 96, 64);
  fillRect(oledc, 0, 0, 96, 63, remapColor(0xff0000, "8R8G8B", "5R6G5B"));
}

If the color value is replaced with the problematic color_value_from block, the code generated is now reasonable.

Screen Shot 2020-05-21 at 5 54 16 PM
// ------ Libraries and Definitions ------
#include "simpletools.h"
#include "ssd1331.h"
#include "colormath.h"

// ------ Global Variables and Objects ------
screen *oledc;

// ------ Main Program ------
int main() {
  oledc = ssd1331_init(17, 6, 5, 3, 2, 96, 64);
  fillRect(oledc, 0, 0, 96, 63, remapColor(getColorRRGGBB(50, 50, 50), "8R8G8B", "5R6G5B"));
}

A workaround for this issue is to assign the value of the color_value_from block to a variable and then assign that variable to the color input of the OLED triangle block

Screen Shot 2020-05-21 at 6 00 39 PM
Automaxion commented 4 years ago

I tried your block-code exactly as you show with 'item' declared first and then used as the color and it still does not work and still gives 'not a number':

// Libraries and Definitions #include "simpletools.h"

include "colormath.h"

include "ssd1331.h"

// Global Variables and Objects screen *oledc; int item;

// Main Program int main() { oledc = ssd1331_init(17, 6, 5, 3, 2, 96, 64); item = (getColorRRGGBB(50, 50, 50)); fillRect(oledc, 0, 0, 96, 63, remapColor(0x NaN, "8R8G8B", "5R6G5B")); }

zfi commented 4 years ago

The "get [color] value from" block is also not returning a 24 bit value as expected. It is instead returning a string that is a function call to convert the selected color. The previous patch applied to the OLED triangle block may work, but that patch will have to applied to each block that expects a color input.

It seems reasonable that these two blocks should be updated to return a 24 bit value.

Screen Shot 2020-05-27 at 3 13 14 PM
Automaxion commented 4 years ago

Any idea when this will be resolved, such that I can use these blocks without error?

zfi commented 4 years ago

@Automaxion The update that addresses this issue is running on our staging server, SoloCup. Please try running your project there and let me know if the issue is resolved.

Automaxion commented 4 years ago

In my more complicated program, I use get colour blocks for red and blue and calculate the green level. Using a simplified version as attached, it still does not compile using SoloCup.

simple solocup simple solocup errors
zfi commented 4 years ago

Thanks for the additional feedback. Digging deeper into the problem we discovered that the OLED blocks were incapable of handling input from variables under certain conditions. These blocks expected numeric inputs in the form 0xRRGGBB. Variable values were evaluated to NaN, which was the root issue. The code has been improved and is available for further testing on SoloCup.

PR #450 contains the update for this issue.

Automaxion commented 4 years ago

It appears to work now, both for the simple code, as above and in my project.

Automaxion commented 4 years ago

I hope the update comes soon since using SoloCup keeps failing on download to Flip after compiling. It works once but then I need to restart and reconnect to Launcher (now v1.0.1)

zfi commented 4 years ago

I have created a new issue for the download failures. We'll track it there.

This issue appears to be resolved in v1.5.0-b4.

PropGit commented 4 years ago

@Automaxion - Please see Issue #452 to answer a question about the download problems.

Automaxion commented 4 years ago

When does BlocklyProp Launcher or BlocklyProp Solo (whichever it is) get upgraded to reflect the resolution to the OLED error so I don't have to use SoloCup?

Stephen Bysouth