ChargePoint / wireshark-v2g

Dissector for the V2G Protocols
Other
43 stars 18 forks source link

EV and EVSE limits are not able to be made into columns due to the values being listed as " #50

Closed JeremyWhaling closed 8 months ago

JeremyWhaling commented 9 months ago

I like having the EV and EVSE limits found in charge parameter discovery as well as the current loop (current demand/response) available as columns in Wireshark. This is not currently possible as the values are not broken out like they are for EV target current, EVSE actual current, EV target voltage, EVSE actual voltage, etc. which can be made into columns because those have fields with labels including the proper scale. Right now the limits set by the EV or EVSE are all listed as "v2gdin.struct.physicalvalue.value" or similar for ISO. If I make a column of "v2gdin.struct.physicalvalue.value" I just get a few items grouped by commas, for example "4,5,5,5,25,32767,32767" in current demand request, which isn't usable.

JeremyWhaling commented 9 months ago

Here's a screenshot highlighting the difference, "Voltage: 500" and "Current: 132.6" are able to be set as columns, but EVMaximumVoltageLimit, EVMaximumCurrentLimit, etc. cannot be. decoderExample

chardin-cpi commented 8 months ago

@JeremyWhaling - so, that's because we have to add special decode fields that are actually not part of the protocol itself.

/* Specifically track voltage and current for graphing */
static int hf_v2gdin_target_voltage = -1;
static int hf_v2gdin_target_current = -1;
static int hf_v2gdin_present_voltage = -1;
static int hf_v2gdin_present_current = -1;

and then those have to be computed in the decode

        value = v2gdin_physicalvalue_to_double(&req->EVTargetVoltage);
        it = proto_tree_add_double(subtree,
                hf_v2gdin_target_voltage,
                tvb, 0, 0, value);
        proto_item_set_generated(it);

and then registered as

                /* Derived values for graphing */
                { &hf_v2gdin_target_voltage,
                  { "Voltage", "v2gdin.target.voltage",
                    FT_DOUBLE, BASE_NONE, NULL, 0x0, NULL, HFILL }
                },
                { &hf_v2gdin_target_current,
                  { "Current", "v2gdin.target.current",
                    FT_DOUBLE, BASE_NONE, NULL, 0x0, NULL, HFILL }
                },
                { &hf_v2gdin_present_voltage,
                  { "Voltage", "v2gdin.present.voltage",
                    FT_DOUBLE, BASE_NONE, NULL, 0x0, NULL, HFILL }
                },
                { &hf_v2gdin_present_current,
                  { "Current", "v2gdin.present.current",
                    FT_DOUBLE, BASE_NONE, NULL, 0x0, NULL, HFILL }
                }

There isn't a generic way to do this and so the patch that goes up for this will just do a few that can be caught - feel free to look at the commit after it goes in to see if you want to add more.

cc: @jhart-cpi

JeremyWhaling commented 8 months ago

I think this new commit will do most of what I want. I would like those values for charge parameter discovery too if I could. This could be in a future commit if necessary, as the bulk of my interest is in the current loop.