mcuadros / OctoPrint-TFT

A OctoPrint touch interface for TFT touch modules based on GTK+3
GNU Affero General Public License v3.0
171 stars 80 forks source link

Error displaying temperature profiles #18

Open ururk opened 6 years ago

ururk commented 6 years ago

(I think the underlying problem is with https://github.com/mcuadros/go-octoprint - and possibly higher up (ie, OctoPrint's API). It probably can be handled in https://github.com/mcuadros/go-octoprint. Posting here because I don't know where the problem really is)

I was getting this error:

cannot unmarshal string into go value of type float64...

when trying to choose a PLA profile. I reviewed my Octoprint config.yml file, and two of the int values were stored as strings:

temperature:
  profiles:
  - bed: 100
    extruder: 210
    name: ABS
  - bed: '50'
    extruder: '205'
    name: PLA

Changing the stings ('50' and '205') to ints fixed the issue, but I'm guessing this problem can easily occur as I did not set these values manually, rather OctoPrint's web interface set them.

CJeffyB commented 6 years ago

Did you file this (potential) bug over with OctoPrint at all?

ururk commented 6 years ago

Did you file this (potential) bug over with OctoPrint at all?

No. I can, but first I'll have to see if I can duplicate the problem. It's kind of a hard one - should:

octoprint's API return an int regardless of how the value is stored? mcuardo's go-octoprint library force the values to an int? or OctoPrint-TFT force the value?

CJeffyB commented 6 years ago

I would guess that, if you set the values using OctoPrint's web interface, the error would be an OctoPrint error. But I can't say that I've ever encountered anything like that.

Can you reliably reproduce the error, and, if so, what steps do you use?

ururk commented 6 years ago

OctoPrint: 1.3.6, using Safari 11.0.3 on OS X 10.13.6 (same happens in Chrome)

Via the web, go to:

Settings > Temperatures > Presets

Click the + button

New preset: PLA/PHA Extruder: 180 Bed: 50

Click Save

YAML:

temperature:
  profiles:
  - bed: 100
    extruder: 210
    name: ABS
  - bed: 50
    extruder: 205
    name: PLA
  - bed: '50'
    extruder: '180'
    name: PLA/PHA

I'll file this in octoprint's git repo, if it isn't already - but would it be reasonable to assume that this project would look for this sort of thing and cast the values to int (float?) regardless?

CJeffyB commented 6 years ago

I would think it wouldn't make sense for any given plugin-type project to assume the data being passed from OctoPrint was incorrect. Otherwise, every plugin would have to sanitize/verify every value, each time. If OctoPrint itself needs those values as Int/Float, then the problem is in OctoPrint. If OctoPrint can work with either, then it may be a plugin issue.

But storing some of those values as Int and some as String does not seem like the sort of design decision that would make sense for OctoPrint, so I'm voting "bug"...

ururk commented 6 years ago

I filed a bug report: https://github.com/foosel/OctoPrint/issues/2520

ururk commented 6 years ago

I didn't try to file it as an API issue - my guess is their API routine is to read the config as-is and translate single-quoted values into strings, and so forth. The underlying problem is how they store the data.

foosel commented 6 years ago

Jup, that's a bug alright. Fix will have to wait until 1.3.8 though, 1.3.7 is already frozen and in RC phase.

yetisnack1 commented 6 years ago

No matter what I try with my config.yaml I get the following error when opening the profiles: unmarshal string into Go struct field supportResendsWithoutOk of type bool

ururk commented 6 years ago

@yetisnack1 That might be a different error, but similar in that the config.yml file is storing a value as a string where Octoprint-tft expects a struct (array in JSON?). Maybe you could share your config.yml file (clear out anything that is sensitive)?

The5py commented 6 years ago

Any solution for that or workaround?

The5py commented 6 years ago

@foosel Was if fixed in 1.3.9 ?

foosel commented 6 years ago

@The5py Yes, as clearly stated in https://github.com/foosel/OctoPrint/issues/2520

The5py commented 6 years ago

@foosel well probeblly some other octoprint-tft issue as you are right and I can clearly see that temperature presets are being saved with single quotes still on octoprint-tft when clicking temperature presets I'm getting this error:

"json: cannot unmarshal string into Go struct field SerialConfig.supportResendsWIthoutOK of type bool" probeblly that's another issue, did not find any workaround solution to change in the config.yaml or in the octoprint-tft /ui/temperature.go code

Anyone got any solution or workaround for this issue?

The5py commented 6 years ago

@foosel I can see here - http://docs.octoprint.org/en/master/configuration/config_yaml.html Under "serial" section: Whether to support resends without follow-up ok or not supportResendsWithoutOk: false

Have no idea what's that parameter, can I set the value into 'false' or "false" instead of Boolean false to check if it solves the above octoprint-tft Error displaying temperature profiles which me & @yetisnack1 got? Or it is not recommended to touch this parameter will it intended functionality still work if I'll set it to string?

RJ-Number1 commented 5 years ago

@The5py - I don't think it matters if you change it or not. Octoprint is changing the field from true/false to always/detect here. I haven't been able to figure out how the OctoPrint-TFT code works to think of a solution. I also don't know Go, so I guess I'll need to figure that out as well.

Dryphter commented 5 years ago

Having this issue as well. Pushing the Profiles button throws the json error. OctoPrint 1.3.9. Config.yaml has the temperature profiles without quotes around the values.

temperature: profiles:

Dryphter commented 5 years ago

This is fixed in the fork blondak made: https://github.com/blondak/OctoPrint-TFT

Temperature profiles work with that version.

jharmn commented 5 years ago

I see that the @blondak fork was merged back in March...it would be great to get a new DEB release. I'm still experiencing this after installing from DEB as recommended.