Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing String SOC and SOH for Model 803 #12

Closed
kenny-peak-energy opened this issue Oct 22, 2024 · 5 comments
Closed

Missing String SOC and SOH for Model 803 #12

kenny-peak-energy opened this issue Oct 22, 2024 · 5 comments
Labels
enhancement New feature or request

Comments

@kenny-peak-energy
Copy link

Hi,

Thanks for the library.

It seems that the string state-of-charge and state-of-health are not represented in the 803 model. Also wondering why the name so_c_sf rather than soc_sf?

From the Sunspec JSON:

                    {
                        "desc": "Battery string state of charge, expressed as a percentage.",
                        "label": "String State of Charge",
                        "mandatory": "M",
                        "name": "StrSoC",
                        "sf": "SoC_SF",
                        "size": 1,
                        "type": "uint16",
                        "units": "%"
                    },
impl crate::Model for Model803 {
    const ID: u16 = 803;
    fn from_data(data: &[u16]) -> Result<Self, crate::ReadModelError> {
        Ok(Self {
            n_str: Self::N_STR.from_data(data)?,
            n_str_con: Self::N_STR_CON.from_data(data)?,
            mod_tmp_max: Self::MOD_TMP_MAX.from_data(data)?,
            mod_tmp_max_str: Self::MOD_TMP_MAX_STR.from_data(data)?,
            mod_tmp_max_mod: Self::MOD_TMP_MAX_MOD.from_data(data)?,
            mod_tmp_min: Self::MOD_TMP_MIN.from_data(data)?,
            mod_tmp_min_str: Self::MOD_TMP_MIN_STR.from_data(data)?,
            mod_tmp_min_mod: Self::MOD_TMP_MIN_MOD.from_data(data)?,
            mod_tmp_avg: Self::MOD_TMP_AVG.from_data(data)?,
            str_v_max: Self::STR_V_MAX.from_data(data)?,
            str_v_max_str: Self::STR_V_MAX_STR.from_data(data)?,
            str_v_min: Self::STR_V_MIN.from_data(data)?,
            str_v_min_str: Self::STR_V_MIN_STR.from_data(data)?,
            str_v_avg: Self::STR_V_AVG.from_data(data)?,
            str_a_max: Self::STR_A_MAX.from_data(data)?,
            str_a_max_str: Self::STR_A_MAX_STR.from_data(data)?,
            str_a_min: Self::STR_A_MIN.from_data(data)?,
            str_a_min_str: Self::STR_A_MIN_STR.from_data(data)?,
            str_a_avg: Self::STR_A_AVG.from_data(data)?,
            n_cell_bal: Self::N_CELL_BAL.from_data(data)?,
            cell_v_sf: Self::CELL_V_SF.from_data(data)?,
            mod_tmp_sf: Self::MOD_TMP_SF.from_data(data)?,
            a_sf: Self::A_SF.from_data(data)?,
            so_h_sf: Self::SO_H_SF.from_data(data)?,
            so_c_sf: Self::SO_C_SF.from_data(data)?,
            v_sf: Self::V_SF.from_data(data)?,
        })
    }
}
@kenny-peak-energy
Copy link
Author

Should this also be an Option?

    /// State of Charge
    ///
    /// State of charge, expressed as a percentage.
    ///
    /// Notes: Measurement.
    pub so_c: u16,

@bikeshedder
Copy link
Owner

The code uses heck to convert between CamelCase and snake_case. SoC is quite unfortunate as it stands for so and c when strictly applying CamelCase rules. You're 100% right that this is quite an ugly artifact and soc would be a better choice.

The code generator can be changed so it does translate SoC to soc. Are you aware of any other artifacts that need to be addressed?

@bikeshedder bikeshedder added the enhancement New feature or request label Oct 22, 2024
@kenny-peak-energy
Copy link
Author

Those are the two that I happened to be looking at. "string state-of-charge" (StrSoC) and "string state-of-health" (StrSoH) seem to be missing. I'm not sure what heck would do with those!

{
                        "desc": "Battery string state of charge, expressed as a percentage.",
                        "label": "String State of Charge",
                        "mandatory": "M",
                        "name": "StrSoC",
                        "sf": "SoC_SF",
                        "size": 1,
                        "type": "uint16",
                        "units": "%"
                    },
                    {
                        "desc": "Battery string state of health, expressed as a percentage.",
                        "label": "String State of Health",
                        "name": "StrSoH",
                        "sf": "SoH_SF",
                        "size": 1,
                        "type": "uint16",
                        "units": "%"
                    },

@bikeshedder
Copy link
Owner

Those would become str_so_c and str_so_h.

This points are missing from the models because this library doesn't support nested and repeating groups, yet:

The code generator is mostly done but I haven't come around implementing the actual parsing, yet.

@bikeshedder
Copy link
Owner

I just fixed the code generation of SoH and SoC. The lack of support for nested groups is tracked in #3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants