SAFE-eV / OCMF-Open-Charge-Metering-Format

24 stars 13 forks source link

Power Loss #18

Closed FlorianRieglerKeba closed 11 months ago

MatLemont commented 1 year ago

Hello,

Thanks for your remarks,

Concerning the Workflow, I feel that the problem is we made too quickly a PR targeting directly the master. I think making change cannot be done without changing version, but this has to pass in a dev branch before being reviewed and merged to the master. I did it this way in the fork, but I cannot create yet branch in the original repo. => I suggest creating this branch version1.2.0 for merging Work in Progress, and then accepting/modifying this evolution in the PR to the master.

For the content:

For LC it represents EVSE characteristic data indeed. I saw it was under "Allgemeines", but I did not pay attention to sub-chapter "Benutzerzuordnung". The german redaction is tricky for non german speakers.

I agree that a proper "EVSE characteristics" chapter will make sense.

LC could be optional you're right, AC application justify this.

For LU, I am strongly OK for keeping it optional while declaring that mOhm unit is default unit if LU is not specified.

CL != 0 @ TX=B is a mistake, it has to be 0 indeed, as described in the field description text.

For LU format, we should let free text formatting not forcing symbols representation but allowing it. So all texts mOhm, mΩ, microOhm or µΩ will be ok. The rationale is that with meter's consideration featuring for example small screen, characters screen, ROM optimization (police kept very minimalistic) ... it can be very inconvenient to force symbol representation if someone is asked to display this information on the screen.

mhei commented 1 year ago

I think making change cannot be done without changing version,

I agree, but my point was more that we should accumulate different changes into a single new version. Not a version for every change.

but this has to pass in a dev branch before being reviewed and merged to the master.

I'm not sure whether we need such a full development workflow with different branches. I'd prefer to bundle the main work in master branch and when we agree upon a document release, then we should create a tag and attach the generated PDF file via Github release page.

I think that we don't have so many ongoing works in parallel that would justify document development and release branches.

I could only imagine this becomes necessary once a OCMF 2.0 is released and new additions should be "backported" to the 1.x branch, too.

BTW: I'm strictly against to check in the PDF in the repository as file as done at the moment.

The german redaction is tricky for non german speakers.

Fully agree, hope we get rid of the German version soon.

For LU format, we should let free text formatting not forcing symbols representation but allowing it. So all texts mOhm, mΩ, microOhm or µΩ will be ok. The rationale is that with meter's consideration featuring for example small screen, characters screen, ROM optimization (police kept very minimalistic) ... it can be very inconvenient to force symbol representation if someone is asked to display this information on the screen.

At the moment, the field "RU" also accepts "random" strings - there is a table "Einheiten" but strictly speaking, this table is not referenced anywhere so the RU field is not limiting the content to the units listed in this table.

If we would allow free text, then wouldn't it be harder for any post-processing software to handle this field? To be honest, I did not yet get your point above...

GeWe-alpitronic commented 1 year ago

Hello,

I would like to add the following remarks:

  1. Cardinality of "LC": From our point of view, cardinality of "LC" must indeed be 0..1 in order to continue to support the AC use case (AC charging customers usually bring their own cables and plug them into our type2 outlets. The minimum cabeling that is provided inside the charging station itself is accounted for by a lump-sum method, with approval of the authorities). Furthermore, 4-wire-measurements might be the future at least for high power charging. In this case, the "LC" field will also not be required.

  2. Line loss impedance units: As a general principle I think we should do our best to make OCMF as consistent and coherent as possible. Therefore, all physical quantities should be handled in the same way. Currently, the reading units (field "RU") are strictly required (cardinality is 1..1). In contrast to @mhei 's comment above

    At the moment, the field "RU" also accepts "random" strings - there is a table "Einheiten" but strictly speaking, this table is not referenced anywhere so the RU field is not limiting the content to the units listed in this table.

to me the specifiation of RU

Reading-Unit: Einheit der Ablesung, z.B. kWh, gemäß Tabelle 19: Vordefinierte Einheiten

means that only the predefined units from table 19 are allowed. Therefore, in my opinion we should

mhei commented 1 year ago

Ah indeed, I looked at the PDF file v1.0.1 in which the reference to the table 19 was still missing - sorry for this.

As a general principle I think we should do our best to make OCMF as consistent and coherent as possible. Therefore, all physical quantities should be handled in the same way.

I fully agree.

Currently, the reading units (field "RU") are strictly required (cardinality is 1..1) ... either stick to the current principle: Units are strictly required for physical quantities and must be taken from table 19.

I guess this first way is the easier one to go.

FlorianRieglerKeba commented 1 year ago

The changes should be made in the englisch version.

FlorianRieglerKeba commented 11 months ago

We moved the content of this pull reqeust to a new one https://github.com/SAFE-eV/OCMF-Open-Charge-Metering-Format/pull/25/ All further discussion should be handled there.

SebMath commented 11 months ago

Sorry I might have missed something ... but ... can someone explain to me in simple words where is the difference between this PR and #25 ?

mhei commented 11 months ago

With #25 merged, we should close this one here.