multigcs / riocore

riocore
GNU General Public License v2.0
19 stars 7 forks source link

quadencoderz documentation #3

Closed jschoch closed 3 months ago

jschoch commented 8 months ago

i can't seem to get quadencoderz working, i was able to get it working using the linuxcnc-rio repo.

Can you provide a working example and explain the following:

What does the "name" field do? i tried to name it "myspindle" and it broke a ton of HAL pins. what does the "function" field do? What is the difference between a plugin and a module and why is there both a quadencoderz plugin and module? the module seems to have the smarts for doing the RPS calculations. How might I troubleshoot a non working config. is there a way to "tee" the input to another hal pin so I can see the bit state of the A/B/Z inputs? How is scale used and for which of the "pins"/"nets"/"signals" what are the encoder types, 0..4, which one is full quadrature? If using full quadrature does scale need to be set to the PPR value?

none of this really makes sense in the Readme.md

"rps": {
            "net": "xxx.yyy.zzz",
            "function": "rio.xxx",
            "scale": 100.0,
            "offset": 0.0,
            "display": {
                "title": "rps",
                "section": "inputs",
                "type": "meter"
            }
        },
jschoch commented 8 months ago

I tried just the bitin on the encoder pins but I don't see anything... I can jog and connect linuxcnc so i'm more confused now.

I mapped pin4 (the button on the tang 9k) with a pullup but the value is 0 in the test-gui and pressing it doesn't change the value. is bitin what I think it is? I notice you use vin in the rio.v, is that just legacy naming?

multigcs commented 8 months ago

bitin is an digital input , bitout an digital output, this should work in the test-gui (or i break something :( )

the naming in rio.v is: all pins starts with 'PININ' or 'PINOUT' depends on the direction

all variables that comes from or goes to the interface starts with 'VARIN' or 'VAROUT' depends on the direction, the number after VAR[IN|OUT] is the size of the variable

VARIN1.... = 1bit Variable from module to TX-Frame VAROUT32.... = 32bit (4byte) Variable from RX-Frame to a module

the name after the first underscore is the name of the plugin-instance, and after the 2. underscore the signal name

VARIN32_STEPDIR15_POSITION = 15. plugin, plugintype: 'STEPDIR' with the signal named 'POSITION' VAROUT1_STEPDIR15_ENABLE = 15. plugin, plugintype: 'STEPDIR' with the signal named 'ENABLE'

if there a modifier in the chain, than the names expand after each step (like debounce and invert an input pin):

pin -> PININ_BITIN3_BIT -> PININ_BITIN3_BIT_DEBOUNCED -> PININ_BITIN3_BIT_DEBOUNCED_INVERTED -> TX-Frame


in the old LinuxCNC-RIO, there exist din/dout , vin/vout varnames, that stands for: digital input/outputs and variable input/outputs.

jschoch commented 8 months ago

your bitin generator has the assign backwards

you can see below where I commented out the generated verilog and reversed it it is working, and the pin where I didn't is not working. It complained about the wire not having a driver and then I realized it was assigning backwards.


//assign PININ_BITIN5_BIT_DEBOUNCED = VARIN1_BITIN5_BIT;
    assign VARIN1_BITIN5_BIT = PININ_BITIN5_BIT_DEBOUNCED;

    // Name: encA_ (bitin)
    //assign PININ_BITIN6_BIT = VARIN1_BITIN6_BIT;
    assign VARIN1_BITIN6_BIT = PININ_BITIN6_BIT;

    // Name: encB (bitin)
    //assign PININ_BITIN7_BIT = VARIN1_BITIN7_BIT;
multigcs commented 8 months ago

the name field in json is for the generated hal pin name:

{
    "type": "quadencoder",
},

becomes: rio.quadencoder2.position and rio.quadencoder2.position-s32

{
    "type": "quadencoder",
    "name": "jog_x",
},

becomes: rio.jog_x.position and rio.jog_x.position-s32

            "type": "quadencoder",
            "name": "jog_x",            "signals": {
                "position": {
                    "net": "axis.x.jog-counts"
                }
            }

generates net entrys in the hal:

net rios.riof_jog_wheel_x <= rio.jog_x.position
net rios.riof_jog_wheel_x => axis.x.jog-counts

for simple assignments

unfortunately, only a few constellations can be mapped in this way, so there is also the 'function' field, thats special keywords for the rio generator to generate more complex hal configurations:

so this

            "signals": {
                "position": {
                    "function": "jog.wheel_x"
                }
            }

becomes this

net rios.riof_jog_wheel_x <= rio.jog_x.position-s32
net rios.riof_jog_wheel_x => axis.x.jog-counts
net rios.riof_jog_wheel_x => joint.1.jog-counts

net rios.axisui_jog_x <= axisui.jog.x
net rios.axisui_jog_x => halui.axis.x.select
net rios.axisui_jog_x => halui.joint.1.select
multigcs commented 8 months ago

..your bitin generator has the assign backwards: ohh, you are right, why did I overlook this :(

multigcs commented 8 months ago

ok, pinout direction looks ok, only the bitin chain is in the wrong direction, sorry

EIDT: must have happened when I moved the modifiers chain into its own class

multigcs commented 8 months ago

ok, hoppefully fixed. can not realy test at the moment

jschoch commented 8 months ago

Ok, back to trying to document this.

I have this in my config:

{
            "type": "quadencoderz",
            "scale": 360,
            "pins": {
                "a": {
                    "pin": "33",
                    "pullup": false
                },
                "b": {
                    "pin": "30",
                    "pullup": false
                },
                "z": {
                    "pin": "29",
                    "pullup": false
                }
            },
            "signals": {
                "indexenable": {
                    "net": "spindle.0.index-enable"
                },
                "indexout": {
                    "net": ""
                },
                "position": {
                    "net": "spindle.0.revs"
                },
                "rps": {
                    "net": "spindle.0.speed-in"
                }
            },
            "name": "spindle"
        },

starting linux cnc with the generated ini throws these errors (but i have to try to start it 10 times because it stops at the first error it finds)


    Pin 'rio.spindle.indexenable' does not exist

    Pin 'rio.spindle.position' does not exist

custom_postgui.hal:11: Pin 'rio.spindle.indexout' does not exist

custom_postgui.hal:18: Pin 'rio.spindle.rpm' does not exist

custom_postgui.hal:24: Pin 'rio.btn_2.bit' does not exist

What in the json/gui creates a HAL pin?

Also, I finally can see a RPS value. it is negative and way off. I'm assuming I need to assign a scale, but if I think about it for a second the RPS should use the index pin (if available) and shouldn't care about how many PPR or CPR the encoder is or what the position is...

Looking at the code it seems there is a duration value that is changed using via the buggy/slow rtapi_get_time() function and the position value. The problem with this is that if the position doesn't update the velocity doesn't update. The LCNC software encoder module has a timeout so that if the spindle stops the velocity decays to 0 because it doesn't see any updates. This is needed and I can take a stab at implementing it and doing a PR.

I'd like to suggest again that we send a timer counter value along with the Z pulse signal and use that to calculate the velocity. We can do this at a slower clock than sysclk to reduce the size of the counter required.

multigcs commented 8 months ago

i think you forget to run halcompile after changing the config

if you have a running linuxcnc, you can browse over the hal-pins in: Machine -> Show Hal Configuration https://linuxcnc.org/docs/html/hal/halshow.html

all hal-names that starts with rio. are the pins from the rio.c hal-component.

the rio. hal-pins are generated by rio.c (needs halcompile):

rio.PLUGINNAME.SIGNALNAME

about the rps, yes, it needs a rewrite, don't know if a timestamp is realy needed (additional data size), but if there is no other way....

jschoch commented 8 months ago

think you forget to run halcompile after changing the config

shit, some warning would be nice for this. newbs are going to be constantly having this issue.
Would you be open to some connection preamble that can check the hash or something?

something like

if !connected
   send_hello()
   wait_for_hello_back()
       get_hash()
       parse_hash()
      if that_hash == my_hash
          connect
     else
         warn(" you need to halcompile or something else is wrong dummy")

What in the json/gui creates a HAL pin?

so for a bitin, choosing name "foo" will create a hal pin called "rio.foo.bit" if you don't pick a name it assigns a generated "rio.bitin.bit" pin I can try to start drafting some documentation in my fork

about the rps, yes, it needs a rewrite, don't know if a timestamp is realy needed (additional data size), but if there is no other way....

I'll play around in a fork and see if I can improve it... I'm not really up to speed on python or how your generator works so I may have to work on the generated outputs (rio.c rio.v, quadencoderz.v)

the "signals" "nets" and "functions" stuff hurts my brain, but I can't even do much hal by hand and your explanation seems like magic since I don't understand it well.

jschoch commented 8 months ago

there is no option for scale in the gui for encoderquadz rps. can I do this via function somehow?

multigcs commented 8 months ago

so for a bitin, choosing name "foo" will create a hal pin called "rio.foo.bit" if you don't pick a name it assigns a generated "rio.bitin.bit" pin yes, but without an name it will generate something like this: 'rio.bitin2.bit' the number of the plugin, so you can have multiple bitin plugins

I can try to start drafting some documentation in my fork that would be very nice !!!

forget the 'functions' field, this is experimental and is no freetext or something from linuxcnc, i have to change the setup gui and use a combo-box for this.

i will add a scale field to setup-gui, at the moment you have to add it in the json file:

                    "signals": {
                        "position": {
                            "scale": 600
                        }
                    }
jschoch commented 8 months ago

I don't think position scale is intended to work for spindle encoders, but I put my scale there manually. It seems that over 600RPM it fails and all the velocity counters go to zero.

I'm going to get my motor config updated and see if I can run some G96 test code. If that works i'll then start messing with using the Z pulse for the velocity calculation and see if the FPGA sending a timer is worth it or not, and drafting some docs.

jschoch commented 8 months ago

I'm muddling through learning the plugin system and drafting some documentation here:

https://github.com/jschoch/riocore/blob/index_velocity/DEV_README.md

and modifying the quad enc module here:

https://github.com/jschoch/riocore/tree/index_velocity/riocore/plugins/quadencoderz2

is there a practical limit for the packet sizes or is it the MTU?

multigcs commented 8 months ago

ahh, cool !

the limit is the duration TX+RX and depends on the interface and the number of plugins, we have to stay below 1ms , faster is better. MTU limit is much too high

jschoch commented 7 months ago

back to the topic.

It appears that spindle.0.speed-in needs to be a positive value. You can do this with abs.N.in and abs.N.out but maybe better to do in the component?

multigcs commented 7 months ago

better in component, it's faster and the hal is cleaner

jschoch commented 7 months ago

I was looking at liteX's issues,

/** 
 * Number of cycles used to calculate the average speed. The minimum speed of the
 * encoder which can be detected whilst using this method is:
 *    1 / (LITEXCNC_ENCODER_POSITION_AVERAGE_SIZE * ENCODER_PPR) * Frequency
 * For example, when using a 3000 PPR Encoder and remembering 8 positions and a 
 * frequency of 1 kHz, this results in 1/24 rotation per second. We have to take
 * into account that at these low speeds the resolution of the speed is detoriated.
*/

maybe also worth checking a minimum speed.

Looks like they are dealing with a lot more corner cases

https://github.com/Peter-van-Tol/LiteX-CNC/blob/36d819f1bbe41e340767121bbac4868562d533ac/src/litexcnc/driver/modules/litexcnc_encoder.c#L264

jschoch commented 7 months ago

It also appears that if the encoder is going the wrong way G33/G76 do not work. Could be useful if there there a way to swap the A/B channels.

multigcs commented 7 months ago

you can swap the pins in json config

multigcs commented 7 months ago

or we can add a new halpin rio.XXXX-inv for inverted values EDIT: and an rio.XXXX-abs halpin

multigcs commented 7 months ago

i have added this new pins to the component generator ..-abs / ...-u32-abs

jschoch commented 7 months ago

This morning i futzed around with rio.c. While the cpu_khz sounds great on paper it also seems that if the cpu frequency changes it isn't reliable. Changing the rtapi_get_time timestamps to long long helped fix the speed > 500 rpm issue but it never seems to report a rps < 800, lower speeds just get set to 0 without any lowpass. G33 is not working correctly and is off by a factor of at least 100. Do you have a working configuration for G33?

I looked at both litex and the mesa encoder stuff and it doesn't seem like either is using this time value or anything from rtapi.

https://github.com/LinuxCNC/linuxcnc/blob/0799d07e34549928d3afe97c5c09c2a68976ec8e/src/hal/drivers/mesa-hostmot2/encoder.c#L936

the software encoder just adds the period ever time update is called

https://github.com/LinuxCNC/linuxcnc/blob/0799d07e34549928d3afe97c5c09c2a68976ec8e/src/hal/components/encoder.c#L401

Are there other implementations to look at? I wish I could understand all this better so I could fix it myself :(

jschoch commented 7 months ago

g33 is fine, my scale param reset to 1 on restart and i forgot to set it again.

still wondering about timestamping...

multigcs commented 7 months ago

i'm not on realtime system at the moment and the rtapi_get_time() is sometimes jumping around :( overall it looks like it works. but, for slower speeds or lower resolution encoders, i need to change some things.

a new Video, at the end, you see the RPS Display running: https://www.youtube.com/watch?v=_VtCqbGNx7M

jschoch commented 7 months ago

i'm a bit scared to try steel, here is machinable wax (paraffin + LDPE ( recycled zip-lock bags))

https://youtube.com/shorts/0T3lXd2zIfU

jschoch commented 7 months ago

Wondering if you can help walk me through your thinking on this code. I've added some comments/questions inline below

void convert_sigin_spindle11_position(data_t *data) {

    // this is the value from the fpga
    float value = data->VARIN32_QUADENCODERZ1_POSITION;
    float offset = *data->SIGIN_SPINDLE11_POSITION_OFFSET;
    // this should b just spindle_scale  position_scale is something else
    float scale = *data->SIGIN_SPINDLE11_POSITION_SCALE;
    // this is the last scaled position value (value/scale)
    float last_value = *data->SIGIN_SPINDLE11_POSITION;
    // why float, see below.
    static float last_raw_value = 0.0;
    // why float?  should be an int from the FPGA correct?
    float raw_value = value;
    // scale and offset for LCNC
    value = value + offset;
    value = value / scale;
    *data->SIGIN_SPINDLE11_POSITION_S32 = value;
    *data->SIGIN_SPINDLE11_POSITION = value;

    // calc rps

    // not sure why this is here?
    float value_rps = *data->SIGIN_SPINDLE11_RPS;
    // if ppr is 360 and 1 pulse has happened since the last update this should be something like:
    //  (100 - 99) * ~1000 (1ms) / 360
    // if more than one update happen between pulses then how does this work?
    // shouldn't this whole function need a  timebase accumulator so that time beyond one update can be accounted for
    // and shouldn't this only get called when there is a pulse?  Then there is the spindle stop issue above.
    // seems like this would result in (100 - 100) * duration / scale == 0
    value_rps = (raw_value - last_raw_value) * *data->duration / scale;
    // set the pin value
    *data->SIGIN_SPINDLE11_RPS = value_rps;

    // calc rpm
    float value_rpm = *data->SIGIN_SPINDLE11_RPM;
    value_rpm = (raw_value - last_raw_value) * *data->duration * 60.0 / scale;
    *data->SIGIN_SPINDLE11_RPM = value_rpm;

    // update static last_raw_value for next iteration's test
    last_raw_value = raw_value;
}
multigcs commented 7 months ago

I'll have to take a closer look at your comments, but first of all:

the functions are very generic float because almost everything is float a copy of existing variables: to make it easier to write code within the plugins

multigcs commented 7 months ago
// this is the value from the fpga

VARIN...: comes from the FPGA/Interface SIGIN...: goes to the signals (halpins in the linuxcnc hal)

// not sure why this is here?

because the functions are generic, all dynamic variables are set to a 'static' name, so you can write simple convert functions inside the plugin.py files like in 'riocore/plugins/freqout/plugin.py':

    def convert_c(self, signal_name, signal_setup):
        return """
        if (value != 0) {
            value = OSC_CLOCK / value;
        }
        """

that looks confusing in the hal functions but is much easier in the plugin

// if more than one update happen between pulses then how does this work?
// shouldn't this whole function need a  timebase accumulator so that time beyond one update can be accounted for
// and shouldn't this only get called when there is a pulse?  Then there is the spindle stop issue above.
// seems like this would result in (100 - 100) * duration / scale == 0

you are right, need a rewrite for this this works in my tests, because i have a 600ppr encoder and my spindle can not run very slow :)

jschoch commented 7 months ago

I'll have to take a closer look at your comments, but first of all:

the functions are very generic float because almost everything is float a copy of existing variables: to make it easier to write code within the plugins

both the litex and mesa encoder code is like 800+ lines. this may something you want a monolith for vs a dynamic plugin.

I also wonder if a monolith isn't generally better for the c component, dynamic generation seems to add a huge amount of complexity. I don't think I could write a full riocore module because of this. and if I have to DIY something it gets blown away every change making any change very fragile. What can't be done in the module with ifdefs and more self describing data?

jschoch commented 7 months ago

I think I found a problem

image

multigcs commented 7 months ago

looks like a combination of pullup resistor and capacitor, do you have the encoder on one of the HDMI pins ? https://wiki.sipeed.com/hardware/zh/tang/Tang-Nano-9K/assets/clip_image010.gif the yellow pins, i have removed the row of little capacitors near the hdmi port

jschoch commented 7 months ago

it is the bob, the 3.3v pullups direct to the tang 9k work fine.