search

sunspec / models

SunSpec Model Definitions
Apache License 2.0
84 stars 52 forks source link

model 803 repeated string group count should be top level NStr #248

Open sv3ndk opened 7 months ago

sv3ndk commented 7 months ago

Model 803 has a repeated group named string, which currently has its count attribute incorrectly set to 0 within the json specification file.

According to the device information model specification, such count attribute should be (section 4.1.2):

defined as a constant in the point group definition or be specified as the value of another point in the model definition. If the point group count is specified in another point, that point MUST be defined in the top-level point group of the model before the point group definition.

As far as I understand, the current 0 value works with pysunspec2 because of a feature deducing the number of repetitions in the group based on the group length and the parent group lenght.

In the case of model 803, the strings group will be repeated once per spring, s.t. count should be NStr: the number of string in the bank.

This PR fixes the model 803 json specification specification, in a similar way as is done in other models (e.g. model 705)

cla-bot[bot] commented 7 months ago

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @sv3ndk on file. In order for us to review and merge your code, please contact the project maintainers to get yourself added.

Kudrat9 commented 7 months ago

Hi sv3ndk,

Model 803 is one of the legacy models that was written in XML. The old format is indicated by defining the repeating block with a count of 0 in the model definition. It is constructed based on the rules of legacy models. And any change to it would cause issues with backward compatibility.

sv3ndk commented 7 months ago

Hi Kudrat9,

Thanks for the explanation, I now understand why we can't change this model specification file.

Since this behaviour needs to be maintained for legacy models, how about it be mentioned in the specification pdf document (or maybe it's there and we missed it?)? Our team is currently using those files outside of pysunspec2, in a server-side library, and were surprised to stumble upon those 0 values.

Kudrat9 commented 7 months ago

Hi sv3ndk,

I will check with my internal team and get back to you.