Clearer outputs for different field types in the CLI and in a JSON export

[chinezbrun]

It is difficult to understand why is not the same approach between raw_value and value ?

From my understanding raw_value is coming directly from mate3 "as is" without any conversion and value is after conversion, like "ready to use" ex. Case 3 with voltage. Which is good!
Now, when we are looking on Case 1 raw_value it is not "as is" and value is not "ready to use".
If I wanted to use JSON data, I would have expected something like that:
raw_value: 1
value: "Enabled"

Case 1
"enable_dhcp": {
"implemented": true,
"scale_factor": null,
"raw_value": "<enable_dhcp.DHCP Enabled: 1>",
"value": "<enable_dhcp.DHCP Enabled: 1>"

Case 2
"tcpip_address": {
"implemented": true,
"scale_factor": null,
"raw_value": "192.168.0.150",
"value": "192.168.0.150"

Case 3
"hbx_grid_connect_voltage": {
"implemented": true,
"scale_factor": -1,
"raw_value": 492,
"value": 49.2

[kodonnell]

Hi @chinezbrun - the above is expected behaviour. Basically, unless it's a scaled integer value (like hbx_grid_connect_voltage) then value will be the same as raw_value.

Now, when we are looking on Case 1 raw_value it is not "as is" and value is not "ready to use". If I wanted to use JSON data, I would have expected something like that: raw_value: 1, value: "Enabled"
...
"raw_value": "<enable_dhcp.DHCP Enabled: 1>",
"value": "<enable_dhcp.DHCP Enabled: 1>"

Firstly, wanting just "Enabled" is unlikely to be implemented as the value "DHCP Enabled" is what's in the Outback spec, and we just automatically convert the spec into the field names etc. So, to get what you want we'd have to manually name all the hundreds (thousands?) of fields, which would be very painful.

Secondly, the above is just the standard representation of python Enum. So, if you have a particular requirement, you can represent it however you want. For example if you want to split the name and value, enums do that for you:

>>> from mate3.sunspec.models import OutBackModel
>>> value = OutBackModel.enable_dhcp.options(1)
>>> value.name
'DHCP Enabled'
>>> value.value
1

However, if you're using this in your code for conditions etc., then you probably should use enums, as this is exactly what they're for - something like:

if mate3.enable_dhcp.value == mate3.enable_dhcp.field.options["DHCP Enabled"]:
    # ...

Using strings (or the raw numbers themselves) would probably not be the pythonic way.

[chinezbrun]

Thanks for you comments.

Firstly, wanting just "Enabled" is unlikely to be implemented as the value "DHCP Enabled" is what's in the Outback spec, and we just automatically convert the spec into the field names etc. So, to get what you want we'd have to manually name all the hundreds (thousands?) of fields, which would be very painful.

true. is was my mistake in that example, value should be "DHCP Enabled" and raw_value = 1, nothing different than Outback spec.

Anyhow the whole idea was to have the export file with raw_values, values exactly in the same format like in Outback spec. Easy to be used in other applications / scripts, not necessarily Python.

[kodonnell]

Ah, right, got you - you want:

"raw_value": "<enable_dhcp.DHCP Enabled: 1>",
"value": "<enable_dhcp.DHCP Enabled: 1>"

To become

"raw_value": 1,
"value": "DHCP Enabled"

My initial reaction is to agree with you as it makes sense, but also that I think it needs more consideration. For example:

1. raw_value is really just meant to be a pointer to the .raw_value python attribute - I never intended people to access it, so it might even be better just to hide it!
2. Enums aren't the only problem - for example, what would you want to happen for bitfields? They have multiple "ready to use" values - so should we show a list of these (as strings or ints?), or just the raw integer for the bitmask - and which as value and which as raw_value? Even the IP address above - should that be stored as the raw bytes or the string value? I guess what I'm saying is if I dig into the enums, then I probably need to think harder about others too.
3. I'm not sure how we implement this. Initial thoughts are that for each Field class we have a to_dict method, which can vary, so we're not limited by just raw_value and value. So, for an enum, it might be (ignoring other things like type/field name/etc.): {"name": "DHCP Enabled", "value": 1} whereas for a 'float' it might be {"integer": 100, "scale_value": -1, "value": 10.0} or for a bitfield it might be {"mask": 6, "set_fields": [ {"name": "a", "value": 2}, {"name": "b", "value": 4}] } etc.
4. As above, my preference is not to support specific formats of outputs, as then it adds maintenance and makes changing harder - i.e. if someone has a specific output requirement, they can just format it themselves (or, if you want to avoid python, use more standard multi-language approaches). Obviously, if we're doing a read to json anyway, then it should be as useful as possible out of box, and I think what you're suggesting makes sense.

@adamcharnock I think this needs direction from you as the owner. Do you want the CLI (including reading etc.) to be

1. Used as an interactive tool (e.g. to quickly check the state of your system) but that's all. In this case, the reading into json can effectively be viewed as example usage, but if people want something different, they can fork it themselves etc. The CLI could be viewed as an example in general.
2. An API itself i.e. that can then be consumed/used by other processes to generate input for them. In this case, the reading into json would need to have a standardised format/specification, etc. as per above.

I don't know if I have a preference, except that, for now, the first is easiest to develop/maintain = ) @chinezbrun - I'm right in thinking you prefer the second?

[chinezbrun]

you guys did a good job here, but if I had to choose I would choose option 2 of course. simple to use, standard export in line with Outback spec.
and yes, export should look like this:

"raw_value": 1,
"value": "DHCP Enabled"

In fact, why I need to read/write Mate3/3s data? to have them recorded in local database for better graphs / monitoring, to used them in various automation / integration (not always python). Of course, in this case, the write has the same importance to me...

[adamcharnock]

Thank you for raising this @chinezbrun, and thanks for fielding it @kodonnell. I'm happy to weigh in.

For me, a CLI such as this is first and foremost a human tool. I say this as I think about other command line tools I use. The pattern seems to be that the default mode of interaction is geared to playing well with actual humans. However, there is normally a way to change into a more machine-readable format.

As another point – and excuse me if I'm restating the obvious here – I think showing any objects which have been repr()ed is the worst of both worlds (i.e. <enable_dhcp.DHCP Enabled: 1>). I propose that it is best to display either the human version or the machine version. This repr()ed version is kinda weird for humans, and a pain for software to parse.

---

My gut feel on this is to have a couple of filters which the output can be put through prior to printing. The default is the human version which prints out nice human-readable values, and perhaps in a non-json format too. Just something easily readable by normals.

The second is the machine-readable filter. This converts the various data types to their raw values and formats the output as JSON. This can be enabled with --export or --output=json or some such.

---

I'm writing all of this having not actually used the post-refactoring version of this codebase yet. While I feel pretty qualified to make high-level software design statements, definitely take any implementation specifics with a pinch of salt.

[kodonnell]

Cool, thanks for the direction @adamcharnock - I think that's clear enough. For the machine readable side of things, I think the next step is to define the json spec (for all field types) and implement a to_json method on all field types (or something similar), which should be pretty trivial. For the human readable aspect of the CLI, I'd likewise start with 'wireframes' for the interface.

I don't use the CLI or the JSON output,and if I wanted something similar I'd just use the API - so the human or machine readable format doesn't worry me. That's a subtle way of saying maybe it's something you'd like to tackle @chinezbrun ?

As another point – and excuse me if I'm restating the obvious here – I think showing any objects which have been repr()ed is the worst of both worlds ...

Hah, it's mostly just repr 'cos that was the easy/lazy option and avoids all the difficulties I mentioned above, and works for the integration tests!

I'm writing all of this having not actually used the post-refactoring version of this codebase yet. While I feel pretty qualified to make high-level software design statements, definitely take any implementation specifics with a pinch of salt.

The new code shouldn't (I hope!) introduce any issues - as above, you to add to_json methods, or override __repr__, or just do bespoke formatting when you hit mate3 read etc.

In fact, why I need to read/write Mate3/3s data? to have them recorded in local database for better graphs / monitoring, to used them in various automation / integration (not always python). Of course, in this case, the write has the same importance to me...

Part of the reason I'm not going to tackle this (at least for now) is I need to spend less time on this code base and more time writing my monitoring system! Though mine will just be using the raw python API (just grabbing the value of a few fields I care about), hence why the output format doesn't worry me.

Re the non-python aspect, there's possibly something to be said for having an example bit of code (or integrating it directly into the library) for e.g. creating a web server that responds to get/post requests, and then any language/device (Alexa?) that speaks HTTP can interface with it. Though maybe that should be a separate project, and we keep this as just the API. Anyway, off-topic!

[kodonnell]

Note that I created #27 which is quite linked to this but is focused on removing the unnecessary confusion (that I created), while this issue has now become more focused on the general CLI and outputs etc. (hence the name change).