Unable to read EEPROM - Githubissues

michaelbaisch commented 4 years ago

Hello,

I'm trying to get my TOM working with OctoPrint. It runs the most recent (for TOM) Sailfish firmware v4.7. I selected TOM Mk7 - single extruder. Not having the "Sailfish option" like in ReplicatorG.

I can connect and jog the printer. Now I wanted to edit the EEPROM values. When I click on EEPROM Settings… I'm getting this message:

Unable to read EEPROM settings. Perhaps because I don't have a matching EEPROM map for your firmware type or version.

The octoprint log says:

2019-11-22 21:45:03,310 - octoprint.plugins.GPX - INFO - batcheeprom
2019-11-22 21:45:03,334 - octoprint.plugins.GPX - WARNING - Unrecognized firmware flavor or version.
2019-11-22 21:45:03,334 - tornado.access - WARNING - 400 POST /plugin/GPX/eeprombatch

I'm also seeing this in the plugin_GPX.log, not sure if this is connected to anything:

Configuration syntax error in /.octoprint/data/GPX/gpx.ini: unrecognized parameters

Greetings :) Michael

michaelbaisch commented 4 years ago

The fundamental Problem seems to be that there is no eeprom map for sailfish v4.7 in GPX. I tried to add one for v4.7 and I ran into a few problems (with GPX but also with OctoPrint-GPX). For now I'm just focusing on the home offset because this is what I have to be able to change:

The values arriving at the requestEepromSettings seem to be unsigned (no matter if et_long or et_ulong was chosen in the eeprom map). Some of my home offset values are negative so I'm getting values like 4294964755. For some reason shifting the number by 0 bits makes js aware of this and makes a signed value out of it. So I kind of fixed it with this change, but I'm really not sure if this is the spot or the right method to fix it.

self.home[axis]((self.eeprom["AXIS_HOME_POSITIONS_STEPS_" + axis]()>>0) / self.steps_per_mm[axis]());

Steps per mm: Apart from the fact that v4.7 uses a 64 bit integer and there is no function to read that in GPX, it also uses another factor. v7.7 uses 1000000 but v4.7 uses 10000000000. Right now this value is hard coded in several places, so no clean way to change it. To get it to work for me I hardcoded my values in self.steps_per_mm[axis] (replicatorG does something similar and just uses the values from the config xml), but this is also not a universal solution.

Any thoughts welcome :)

BTW eeprom maps for different sailfish version can downloaded here sailfish_makerware_eeprom_maps.zip.

Greetings, Michael

markwal commented 3 years ago

Hmmm...

a. 64-bits At the bottom, the x3g protocol uses https://github.com/makerbot/s3g/blob/master/doc/s3gProtocol.md#12---read-from-eeprom which reads an indicated number of bits. The documentation claims a maximum read of 31 bytes. GPX on the other hand always requests 2, 4, or n (in the case of a string) bytes. I'm not sure why wingcommander decided to implement it that way actually. It seems like we could add a type for a 64-bit integer though.

b. As for why it comes back as unsigned regardless... py_read_eeprom uses Py_BuildValue to make a signed or unsigned value out of the 32-bits by indicating "l" or "k" as the type and then the python function GPXPlugin.eeprom() sends that python value to flask.jsonify. I'm not sure where the type is getting lost. Your workaround makes sense to me failing fixing the root cause.

Interesting. Yep. I hardcoded the unit scale factor right in there. We probably want to make that variable. Or we could massage it in GPX so that it always looks like 7.7 units and then scales internally before sending/receiving over the wire. That's assuming that the extra precision isn't getting you anything since sailfish removed it in v7.

michaelbaisch commented 3 years ago

b. I looked into the unsigned part a bit more. the values in response in batcheeprom are already signed. So onto py_read_eeprom, I added some debugging prints:

gpxmodule.c:732

            if (read_eeprom_32(&gpx, gpx.sio, pem->address, &ul) == SUCCESS)
                fprintf(gpx.log, "ul (i) %i\n", ul);
                fprintf(gpx.log, "ul (d) %d\n", ul);
                fprintf(gpx.log, "ul (u) %u\n", ul);
                fprintf(gpx.log, "Py_BuildValue (i) %i\n", Py_BuildValue("l", ul));
                fprintf(gpx.log, "Py_BuildValue (d) %d\n", Py_BuildValue("l", ul));
                fprintf(gpx.log, "Py_BuildValue (u) %u\n", Py_BuildValue("l", ul));

Output for negative value:

ul (i) -2541
ul (d) -2541
ul (u) 4294964755
Py_BuildValue (i) 570776720
Py_BuildValue (d) 570769224
Py_BuildValue (u) 570770008

Output for positive value:

ul (i) 22190
ul (d) 22190
ul (u) 22190
Py_BuildValue (i) 570768792
Py_BuildValue (d) 570770032
Py_BuildValue (u) 570769912

batcheeprom receives 4294964755 and 22190. This is not exactly clear to me, it's a weird back and forth between c and python…

michaelbaisch commented 3 years ago

Another thought of mine was, if we could access the sailfish version in js, we could simply use the appropriate scale factor for that version.

markwal commented 3 years ago

1.b. Py_BuildValue returns a pointer to a Python value structure so the numbers you are printing out are addresses in memory.

true

markwal commented 3 years ago

true, though it makes the map abstraction a bit leaky

markwal commented 3 years ago

It is already leaky, but makes it more leaky. Perhaps we should scale to float in the library so that python sees the float. The js is already doing that so the loss in precision is happening regardless.

michaelbaisch commented 3 years ago

Yeah makes sense that those are pointers… If I log in batcheeprom() directly what gpx.read_eeprom(eepromid) returns like:

self._logger.debug("gpx.read_eeprom(eepromid) = %s" % gpx.read_eeprom(eepromid))

I'm getting values like 4294964755. So it kind of seems like Py_BuildValue does something weird?!

michaelbaisch commented 3 years ago

So, you mean that calling eg gpx.read_eeprom(AXIS_STEPS_PER_MM_X) does not return the actual eeprom data but a already scaled float?

markwal commented 3 years ago

Yes, python has to have values in a self describing structure because the value has to also describe type (ie not a strongly typed language)
I'm proposing to change it to be so.

thebeline commented 3 years ago

ETA on this?

markwal commented 3 years ago

@thebeline Not really working on it, to be honest. Not clear how many people out there are interested in eeprom maps other than current Sailfish other than @michaelbaisch. What is it that you are interested in? IE what version of the firmware are you using and what are you trying to do with it?

thebeline commented 3 years ago

I recently acquired a CTC Dual with Sailfish 7.6. Really, for me, I need to be able to adjust the steps/mm as they are currently a touch out of whack, and updating them in the plugin doesn't seem to do anything.

(BTW, VERY impressed how fast you got back to me here...)

markwal commented 3 years ago

The CTC Dual should work theoretically work with the map that we have. I think they did build their own firmware, but I don't think they changed the eeprom layout. What symptoms are you seeing? Do you get the same message as above? What does the log say?

markwal commented 3 years ago

Also, the steps/mm is primarily used by GPX itself to translate from gcode to x3g, so when that when you send a gcode command through OctoPrint, it gets translated using that setting. It doesn't use the eeprom setting for that, just the numbers you store in the plugin settings (stored on the Raspberry Pi).

thebeline commented 3 years ago

One would assume, then, that changing those params would affect a print. Yet they don't really...

The steps/mm with whatever the previous owner has on here, seem to be off by nearly 10% in the Y-axis, and yet, even testing, HALVING the Y-Axis stems/mm doesn't really seem to change anything.

That is... unless you are caching the generated x3g, which... would be unfortunate...

markwal commented 3 years ago

Are you printing x3g from the SD card? Or gcode from the pi?

BTW: The place you want to change the steps/mm is in the "Edit Machine Definition..."

If that's what you're doing and it isn't working, could you paste in your gpx.ini?

ssh into the pi and:

cd ~/.octoprint/data/GPX
cat gpx.ini

thebeline commented 3 years ago

Three Questions:

Should I start a new topic?
Do you cache the converted x3g?
Is there an appreciable delay to editing the Machine Definition and the edits effecting subsequent prints? I was calibrating, so I was editing, then reprinting, all within a few seconds.
In the Machine Definition, are the steps/mm directly or inversely proportional? IE: If I were to edit a Marlin FW, raising the steps/mm would shrink a distance. Would I be correct to assume that in Machine Definition, this behavior would be the same?

markwal commented 3 years ago

Yes, I think you've found a new issue. The machine definition steps/mm appear to not be persisting properly. I'm guessing a regression due to a dependency.
No
Shouldn't be.
steps/mm is just that, how many steps/mm. So when you increase it, the translator will tell the bot to go more steps for the same indicated mm distance. So raising steps/mm will increase the distance.

thebeline commented 3 years ago

Yes, I think you've found a new issue.

Kay.

thebeline commented 3 years ago

I'd still like to be able read/write EEPROM, though... ;-)

markwal / OctoPrint-GPX

Unable to read EEPROM #74