search

ASCOMInitiative / ConformU

ConformU is a cross-platform tool to validate that Alpaca Devices and ASCOM Drivers conform to the ASCOM interface specification. It supersedes the original Windows based Conform application.
11 stars 2 forks source link

Human-readable representation of enums in settings #2

Closed RReverser closed 1 year ago

RReverser commented 1 year ago

Before the 1.0 RC, enums like DeviceTypes were represented in conform.settings like human-readable strings:

"AscomDeviceType": "Camera",
...

In the new version it appears all enums are represented as internal indices in the enum instead:

"AscomDeviceType": 0, // <- represents DeviceTypes::Camera
// ...
"AccessServiceType": 0, // <- represents ServiceType::Http
// ...
"DeviceTechnology": 1, // <- represents DeviceTechnology::Alpaca
// ... and so on

Not sure if this was intentional, but it can be problematic for 1) users who manually customize the settings JSON file and pass it via command-line interface to conformu and 2) might be not future-proof for ConformU itself, in case some enum variants get reordered (unlikely, but still).

I understand using indices is reasonable internal representation, but I think that for the settings JSON file it would be better to switch all enum encodings to strings - the cost is negligible, and the convenience is worth it.

Peter-Simpson commented 1 year ago

Hi Ingvar,

Those are good points. I switched over to using device type as an enum type because it was easier and more reliable to code with than using the original string type. There are several other enum types in addition to those you mentioned that could also benefit with being represented as strings.

I'm using System.Text.Json to serialise the data class and have found the JsonStringEnumConverter() class that I can use to dynamically convert enum values to their string values during serialisation and the reverse during de-serialisation.

I'm pleased to say that it became essentially two "one line fixes" to keep the enum representation inside the application and to persist enums as strings in the settings file.

The change will be in the next release but I'm going to wait a while to see if I get other helpful feedback. If you'd like an early look at the update please can you let me know which OS you are using and I'll send you a link to that installer.

Many thanks, Peter

RReverser commented 1 year ago

Thanks, yeah, conversion at the serialization layer is exactly what I had in mind.

If you'd like an early look at the update please can you let me know which OS you are using and I'll send you a link to that installer.

It's okay, I believe you it works and can wait till the next release :)