search

ConsumerDataStandardsAustralia / standards-maintenance

This repository houses the interactions, consultations and work management to support the maintenance of baselined components of the Consumer Data Right API Standards and Information Security profile.
41 stars 9 forks source link

Iteration 11 Holistic Feedback #511

Closed CDR-API-Stream closed 1 year ago

CDR-API-Stream commented 2 years ago

Description

This CR has been created to simplify the raising of minor changes, such as text corrections or description clarifications, that are not really material to the standards but do have real impact of readability and clarity.

Please raise any such suggestions that you would like included in Maintenance Iteration 11 and the DSB will review them. If we believe the suggestion are material we will raise a dedicated CR for them.

perlboy commented 2 years ago

First comment, this one looks like it could be rolled in: https://github.com/ConsumerDataStandardsAustralia/standards-maintenance/issues/461

perlboy commented 2 years ago

Clarification on Get Agreed Payment Schedule sought during implementation calls has indicated that 404 is not a valid response for situations where an account has no specifically agreed payment schedule and that instead when a Consumer "signs up" to a plan they agree to a payment schedule.

This leads to the following clarifications required:

  1. The description seems like it should be altered to "Obtain the agreed payment schedule and details for a specific energy account" as "if any" implies conditionality
  2. What manualPayment and cardDebit responses look like for situations where the consumer has paid in advance (ie. they get a bill but require no payment)
  3. What the Agreed Payment Schedule is for prepaid energy plans (such as Reamped Advance or Powershop Powerpacks)
  4. Energy Billing systems allow for multiple payment schedules while the payment schedules endpoint only allows for a single value. How should a retailer present this?
  5. What the response should look like if it is a PayPal payment schedule?

There is also an MI item https://github.com/ConsumerDataStandardsAustralia/standards-maintenance/issues/495 for this, happy to move this comment there if it seems more appropriate.

Further update from implementation call for this endpoint:

perlboy commented 2 years ago

Merged enum related components

EnergyBillingPaymentTransaction.method refers to card payments as CARD while EnergyPlanContract.paymentOption refers to CREDIT_CARD. Given both are about acquisition method suggest they be aligned to simply CARD.

EnergyServicePointDetail.timeOfDay doesn't seem to include spaces in enums, notably ALLDAY and OFFPEAK. OFFPEAK in particular is notable because there is OFF_PEAK in other payloads too.

EnergyPlanGreenPowerCharges.scheme doesn't seem to include spaces in enums, GREENPOWER should probably be GREEN_POWER

perlboy commented 2 years ago

The following fields are currently specified in the Standards as number but examples are an integer and they appear to actually be integers in source specifications or underlying standard:

The following fields have implied defaults if not specified, may be integers and DER Guidelines state Default values AS4777-2: 2015 section 7.4 which is a paid document so at a minimum having a description that provides them would be useful:

The following fields seem either non-existent or incorrectly defined:

The following fields have descriptions worth that should probably be fixed with proposals in italics:

perlboy commented 2 years ago

Clarification is requested for oldest-date and newest-date filters:

nils-work commented 2 years ago

It seems like these two * points may have been intended to be bullet points - image

JamesMBligh commented 2 years ago

Via CDR Support Portal. The language for some of the common types (e.g. MaskedPANString) use 'should' inconsistently with RFC2119 as their language predates the use of the RFC. The type descriptions should be reviewed to align to the intent (i.e. make each 'should' an explicit SHOULD or MUST).

nils-work commented 2 years ago

From v1.17.0, I think the anchor link in this section is incorrect, perhaps it should point to - https://consumerdatastandardsaustralia.github.io/standards/#profile-scope-and-standard-claims

image

markverstege commented 2 years ago

The following CRs have been included in Maintenance Iteration 11. Each represents a minor documentation fix. They've been cross posted here within the holistic feedback CR for the iteration. Changes will be staged against each within a maintenance/511 branch as individual commits.

The following CRs will be closed:

CDR-API-Stream commented 2 years ago

From v1.17.0, I think the anchor link in this section is incorrect, perhaps it should point to - https://consumerdatastandardsaustralia.github.io/standards/#profile-scope-and-standard-claims

image

This issue has been fixed. It is staged for review here: https://github.com/ConsumerDataStandardsAustralia/standards-staging/compare/release/1.18.0...maintenance/511. Commit cb9e55d.

CDR-API-Stream commented 2 years ago

It seems like these two * points may have been intended to be bullet points - image

This issue has been fixed. It is staged for review here: https://github.com/ConsumerDataStandardsAustralia/standards-staging/compare/release/1.18.0...maintenance/511. Commit 7b05677.

CDR-API-Stream commented 2 years ago

Issue #487 has been staged for review: https://github.com/ConsumerDataStandardsAustralia/standards-maintenance/issues/487#issuecomment-1136626910

markverstege commented 2 years ago

To improve consistency, bold all Requirement Level keywords usages (e.g. MUST, SHOULD, ...) throughout the standards.

CDR-API-Stream commented 2 years ago

Issue #494 has been staged for review: https://github.com/ConsumerDataStandardsAustralia/standards-maintenance/issues/494#issuecomment-1136671959

CDR-API-Stream commented 2 years ago

Description

This CR has been created to simplify the raising of minor changes, such as text corrections or description clarifications, that are not really material to the standards but do have real impact of readability and clarity.

Please raise any such suggestions that you would like included in Maintenance Iteration 11 and the DSB will review them. If we believe the suggestion are material we will raise a dedicated CR for them.

The baseline review branch for these holistic changes has been staged here: https://github.com/ConsumerDataStandardsAustralia/standards-staging/compare/release/1.18.0..maintenance/511. As changes are addressed in the comments above, they will be incorporated into this staging branch for community review.

CDR-API-Stream commented 2 years ago

Issue #489 has been staged for review: https://github.com/ConsumerDataStandardsAustralia/standards-maintenance/issues/489#issuecomment-1136697800

HemangCDR commented 2 years ago

Specify that the sort order of usage reads in the energy standards (i.e. in EnergyUsageListResponse) will be by NMI (ascending), read start date (descending)

CDR-API-Stream commented 2 years ago

Issue #497 has been staged for review: https://github.com/ConsumerDataStandardsAustralia/standards-maintenance/issues/497#issuecomment-1138090708

perlboy commented 2 years ago

Specify that the sort order of usage reads in the energy standards (i.e. in EnergyUsageListResponse) will be by NMI (ascending), read start date (descending)

@HemangCDR, sorry I added this without realising you already had. For what it's worth, the sort order doesn't appear to have a ratified Decision Proposal supporting it.

Original Post:

Sort order of usage data is not specified. In fact the DP for usage data didn't seem to specify it either. It appears to have last been discussed in an abandoned prior decision proposal.

HemangCDR commented 2 years ago

@perlboy no worries.

DP 195 did have the sort order specified, it was in the "General Notes" section of the proposal as below:

It was missed in the decision document (which is probably why it isn't in the standards). We will ratify it part of MI11.

perlboy commented 2 years ago

@perlboy no worries. DP 195 did have the sort order specified, it was in the "General Notes" section of the proposal as below:

  • "Reads will be sorted by NMI followed by read start date in descending order" It was missed in the decision document (which is probably why it isn't in the standards). We will ratify it part of MI11.

Yes. That was what I was highlighting. The only thing binding is the Standards which are tied to decisions of the Chair and the decision the Chair provided didn't include it. Consequently the sort order isn't actually ratified by the Chair. It should be. 😄

nilsberge commented 2 years ago

Another minor documentation update similar to above, I think these should also be a list - image

OpalRussAEMO commented 2 years ago

Suggested updates to EnergyDerRecord, note some of these overlap with Stuart's suggestions.

In regards to Stuart's comments on the manufacturer and model's potentially being enumerated, these values aren't enumerated in the DER Register so enumeration can't be supported.

OpalRussAEMO commented 2 years ago

Clarification is requested for oldest-date and newest-date filters:

  • oldest-date filter specifies the default value is "If absent defaults to newest-date minus 24 months days". Is this a simple 24 months subtraction (ie. "today" is 2022-05-15 so oldest-date would be 2020-05-15) that ignores other conditions (length of months, leap years etc) or something more complex?
  • What timezone are oldest-date and newest-date to be considered at? UTC seems feasible but clarification is important relative to intervalReads (see below)
  • EnergyUsageRead.intervalRead.intervalReads specifies specified by readIntervalLength beginning at midnight of readStartDate. What is "midnight"? I think this is AEST however making it explicit in the description would be helpful to implementers
perlboy commented 2 years ago
  • EnergyDerRecord.availablePhasesCount - should be a positive integer, confirmed values are only 1,2 or 3. Could be enumerated.

Thanks for confirming this Opal. Any reason why this couldn't be an enum instead? It seems more descriptive is all. Also, does the Australian energy market actually have live 2 phase installs? I thought 2 phase was killed off in the early 1900s?

EnergyUsageRead.unitOfMeasure: can we note that values will be provided in uppercase?

This really sounds like an enum. The table from the NEM 12/13 spec in Appendix B (Page 22) was quite helpful and we wondered whether a universal EnergyUnitOfMeasure (or maybe EnergyMeasurementType) would have a lot of usefulness for implementers trying to correlate data between endpoints.

All dates/times relating to meter data and standing data (service point detail) are recorded in AEST therefore all dates/times for meter data should be interpreted as AEST.

The timezone problem looks like one of the more difficult problems to solve in this sector. While I accept that all AEMO data is currently AEST embedding this timezone will generally preclude other potential meter sources (for instance voluntary data holders that don't participate in the NEM) and plan data has this concept of LOCAL which, while I personally think is a bit silly, is the existing source. I wonder whether EnergyUsageRead should instead have some idea of timeZone with a UTC offset that defaults to UTC+10 (ie. AEST unless otherwise noted). Finally on this I think aligning to UTC offsets is probably more worthwhile for potential international transferability.

CDR-API-Stream commented 2 years ago

Another minor documentation update similar to above, I think these should also be a list - image

This issue has been fixed. It is staged for review here: https://github.com/ConsumerDataStandardsAustralia/standards-staging/compare/release/1.18.0...maintenance/511. Commit 4e285de.

CDR-API-Stream commented 2 years ago

Clarification on Get Agreed Payment Schedule sought during implementation calls has indicated that 404 is not a valid response for situations where an account has no specifically agreed payment schedule and that instead when a Consumer "signs up" to a plan they agree to a payment schedule.

This leads to the following clarifications required:

  1. The description seems like it should be altered to "Obtain the agreed payment schedule and details for a specific energy account" as "if any" implies conditionality
  2. What manualPayment and cardDebit responses look like for situations where the consumer has paid in advance (ie. they get a bill but require no payment)
  3. What the Agreed Payment Schedule is for prepaid energy plans (such as Reamped Advance or Powershop Powerpacks)
  4. Energy Billing systems allow for multiple payment schedules while the payment schedules endpoint only allows for a single value. How should a retailer present this?
  5. What the response should look like if it is a PayPal payment schedule?

There is also an MI item #495 for this, happy to move this comment there if it seems more appropriate.

Further update from implementation call for this endpoint:

  • isTokenised says False if absent. If false then bsb and accountNumber should not be expected to be included but bsb/accountNumber says Is required if isTokenised is absent or false which seems to be conflicting

    • (@perlboy note) This one was added when a representative from a Retailer raised it but does seem to be aligned in the sense if isTokenised is true then bsb/accountNumber would not be expected. The wording though seems to be a double negative. Suggest the following:
    1. isTokenised phrase stated as False if absent. If true then bsb and accountNumber must be included
    2. bsb and accountNumber phrase left unchanged or possibly removed entirely leaving isTokenised qualifier as the source of truth

The above comments have been addressed in CR #495. Please refer to this comment.

CDR-API-Stream commented 2 years ago

In response to @perlboy's comment

The following fields are currently specified in the Standards as number but examples are an integer and they appear to actually be integers in source specifications or underlying standard:

  • EnergyDerRecord.availablePhasesCount: Phases are always whole numbers and according to DER Guidelines it is an enumeration ("Pick List") of [1,2,3] so should probably be an enumeration
  • EnergyDerRecord.installedPhasesCount: Phases are always whole numbers and according to DER Guidelines it is an enumeration ("Pick List") of [1,2,3] so should probably be an enumeration

Based on @OpalRussAEMO's comment, the DSB proposes to make the above attributes PositiveInteger. We can update the description to state the acceptable values are 1, 2 and 3.

  • EnergyDerRecord.protectionMode.exportLimitkva: This field also appears to be incorrectly camelCased as KVA and kVA are interchangeable and KVA is used elsewhere, would be best to be consistent

The above attribute is represented exactly as it is from AEMOs DER specification similar to other attributes.

The following fields have implied defaults if not specified, may be integers and DER Guidelines state Default values AS4777-2: 2015 section 7.4 which is a paid document so at a minimum having a description that provides them would be useful:

  • EnergyDerRecord.protectionMode.underFrequencyProtection: AS4777-2 default is 47Hz (Aus A/B) or 45Hz (Aus C)
  • EnergyDerRecord.protectionMode.underFrequencyProtectionDelay: AS4777-2 default is 1s (Aus A/B) or 5s (Aus C)
  • EnergyDerRecord.protectionMode.overFrequencyProtection: AS4777-2 default is 52Hz (Aus A/B) or 55Hz (Aus C)
  • EnergyDerRecord.protectionMode.overFrequencyProtectionDelay: AS4777-2 has this marked as - and it is missing in the DER Guidelines but exists in the DER Register Technical specification
  • EnergyDerRecord.protectionMode.underVoltageProtection: AS4777-2 default is 180V
  • EnergyDerRecord.protectionMode.underVoltageProtectionDelay: AS4777-2 default is 10s
  • EnergyDerRecord.protectionMode.overVoltageProtection: AS4777-2 default is 265V
  • EnergyDerRecord.protectionMode.overVoltageProtectionDelay: AS4777-2 default is 1s
  • EnergyDerRecord.protectionMode.sustainedOverVoltage: AS4777-2 default is 275V

The default values have been left out in the CDS on purpose after consultations with AEMO. This is mainly because the default values cannot be assumed as there are scenarios where the distributor/installer may not supply the value to AEMO and it is unknown. The underlying specs may also change the default values in the future.

The following fields seem either non-existent or incorrectly defined:

  • EnergyDerRecord.protectionMode.sustainedOverVoltageDelay: There is no trip delay specified for V>> in the DER Guidelines and AS4777-2 has these marked as -. The DER Register Technical specification has this field but it is described as "Sustained Over voltage protection delay in volts (V)" which seems quite wrong (it's a time measurement not a voltage). It's unclear if this field even exists?

The description of the above field in DER spec, is defined as - "Sustained Over voltage protection delay in volts (V)". However, in Guide to DER APIs document it is described as - "Sustained Over voltage protection delay in seconds."

The DSB can update the description of the field based on AEMO's feedback.

  • EnergyDerRecord.acConnections[].derDevices[].manufacturer appears to be mandatory and in fact an enumeration ("pick list") in the DER Guidelines with a comment of "Definitions align to the approved modules list.".
  • EnergyDerRecord.acConnections[].derDevices[]. modelNumber appears to be mandatory and in fact an enumeration ("pick list") in the DER Guidelines with a comment of "Definitions align to the approved modules list."

The fields were not made ENUM based on consultation with AEMO as the values change frequently (more devices models added) and the data provided by the installer may not be consistent.

Clarification is requested for oldest-date and newest-date filters:

  • oldest-date filter specifies the default value is "If absent defaults to newest-date minus 24 months days". Is this a simple 24 months subtraction (ie. "today" is 2022-05-15 so oldest-date would be 2020-05-15) that ignores other conditions (length of months, leap years etc) or something more complex?

The above understanding is correct, it is a simple 24 months subtraction.

  • What timezone are oldest-date and newest-date to be considered at? UTC seems feasible but clarification is important relative to intervalReads (see below)
  • EnergyUsageRead.intervalRead.intervalReads specifies specified by readIntervalLength beginning at midnight of readStartDate. What is "midnight"? I think this is AEST however making it explicit in the description would be helpful to implementers

The oldest-date and newest-date are filters on a fields in a given payload and align to the fields in that payload. For reads, it should be AEST. The description of readStartDate and readEndDate can be updated to specify they are in AEST.

perlboy commented 2 years ago

Based on @OpalRussAEMO's comment, the DSB proposes to make the above attributes PositiveInteger. We can update the description to state the acceptable values are 1, 2 and 3.

👍

  • EnergyDerRecord.protectionMode.exportLimitkva: This field also appears to be incorrectly camelCased as KVA and kVA are interchangeable and KVA is used elsewhere, would be best to be consistent The above attribute is represented exactly as it is from AEMOs DER specification similar to other attributes.

This may be the case but the Standards state:

Field names MUST be defined using camel case with the following clarifications

The justification of aligning to a source document is neither a Standard, a camel case clarification nor makes much sense since the DER specification also includes the following statement:

Any data submitted by the NSPs that is related to the CEC is accepted to be non case sensitive.

By the above interpretation the attribute name for BankingTransactionDetail.x2p101Payload.endToEndId should actually be BankingTransactionDetail.x2p101Payload.EndToEndId because that's what it is in the source documents that define an XML Standard.

Suggest the DSB follow its own Standard regardless of source document or there won't actually be a Standard when you get to Telco.

The following fields have implied defaults if not specified, may be integers and DER Guidelines state Default values AS4777-2: 2015 section 7.4 which is a paid document so at a minimum having a description that provides them would be useful:

  • EnergyDerRecord.protectionMode.underFrequencyProtection: AS4777-2 default is 47Hz (Aus A/B) or 45Hz (Aus C) The default values have been left out in the CDS on purpose after consultations with AEMO. This is mainly because the default values cannot be assumed as there are scenarios where the distributor/installer may not supply the value to AEMO and it is unknown. The underlying specs may also change the default values in the future.

This makes it even more important to treat values AEMO does not know as something other than null because AS4777-2 specifies defaults and now the community is going to be split between who paid for the AS and who only read the Standards.

Suggest instead of a null or missing AEMO provides a -1 or 0 value (which isn't valid for any of the number fields so will not conflict) and it is designated as "Unknown" or similar.

The description of the above field in DER spec, is defined as - "Sustained Over voltage protection delay in volts (V)". However, in Guide to DER APIs document it is described as - "Sustained Over voltage protection delay in seconds." The DSB can update the description of the field based on AEMO's feedback.

👍

The oldest-date and newest-date are filters on a fields in a given payload and align to the fields in that payload. For reads, it should be AEST. The description of readStartDate and readEndDate can be updated to specify they are in AEST.

That would be preferred because it is impossible to state that all interval reads will always be in AEST because payloads may not always be served by AEMO (eg. other non-included states may be different or voluntary data holders providing metering data from consumer solar installations).

Also, can I refloat the question around making unitOfMeasure an enumeration? There would be a lot of benefit to having this and @OpalRussAEMO already indicated the intent would be to uppercase these values anyway.

CDR-API-Stream commented 2 years ago

To improve consistency, bold all Requirement Level keywords usages (e.g. MUST, SHOULD, ...) throughout the standards.

These changes have been staged for review here: https://github.com/ConsumerDataStandardsAustralia/standards-staging/compare/maintenance/511...maintenance/511-requirement-levels.

Given the large number of changes they have been staged under a separate feature branch off maintenance/511.

perlboy commented 2 years ago

These changes have been staged for review here: ConsumerDataStandardsAustralia/standards-staging@maintenance/511...maintenance/511-requirement-levels. Given the large number of changes they have been staged under a separate feature branch off maintenance/511.

Can you click the Create PR Request button please so that an inline review can happen. Otherwise this thread is going to get crazy long.

Edit: I actually question whether doing this change in a feedback thread is appropriate. Converting to binding 2119 should be treated as a task in itself. Prior "clarification" changes the DSB have made have blown up in implementers faces, it would be advisable for the DSB to learn from this mistake.

CDR-API-Stream commented 2 years ago

Hi @perlboy, done: https://github.com/ConsumerDataStandardsAustralia/standards-staging/pull/195

Given the changes are staged in a separate branch we can discuss the treatment of them in the next Maintenance Iteration call. This review was conducted under the normative standards review and could be considered under that work: https://github.com/ConsumerDataStandardsAustralia/standards-maintenance/blob/master/reviews/2021-05/analysis/analysis-rfc2119-rfc8174-20210519.md

perlboy commented 2 years ago

https://github.com/ConsumerDataStandardsAustralia/standards-staging/pull/195#pullrequestreview-1003723299

Repasting what I wrote as a comment in the PR Review:

There seems to be non 2119 terms used which is not appropriate. Additionally there seems to be excessive 2119 language in use. While this may be a first draft what is proposed makes significant changes to the underlying Standards as they are understood at present. I've walked through as much as I can to provide assistance but this needs a lot more work.

I think it's appropriate to quote 2119 here as the approach taken by the DSB seems to be one of mass search and replace:

Imperatives of the type defined in this memo must be used with care and sparingly. In particular, they MUST only be used where it is actually required for interoperation or to limit behavior which has potential for causing harm (e.g., limiting retransmisssions) For example, they must not be used to try to impose a particular method on implementors where the method is not required for interoperability.

It's a pretty big line ball as to whether many of the 2119 statements proposed actually meet this requirement or are simply commentary dressed up as Standards.

CDR-API-Stream commented 2 years ago

Below is a list of all the changes the DSB will recommend be made to the DER structure discussed within this CR:

CDR-API-Stream commented 2 years ago

Below is a list of all the changes the DSB will recommend be made to the DER structure discussed within this CR:

  • EnergyDerRecord.availablePhasesCount - Make the attributes PositiveInteger and update the description to state the acceptable values are 1, 2 and 3.
  • EnergyDerRecord.installedPhasesCount - Make the attributes PositiveInteger and update the description to state the acceptable values are 1, 2 and 3.
  • EnergyDerRecord.acConnections.derDevices.count - Make the attribute PositiveInteger
  • EnergyDerRecord.islandableInstallation - Make the attribute Boolean
  • EnergyDerRecord.protectionMode.exportLimitkva - Change it to camelCase - exportLimitKva
  • EnergyDerRecord.protectionMode.sustainedOverVoltageDelay - Based on feedback from AEMO, update description to "Sustained Over voltage protection delay in seconds."
  • EnergyUsageRead.readStartDate - Update description to specify it is AEST and assumed to start from 12:00 am. Remove remove reference to time from description
  • EnergyUsageRead.readEndDate - Update description to specify it is in AEST and remove reference to time
  • EnergyUsageRead.meterID - Change to meterId (lowercase d)
  • EnergyUsageRead.unitOfMeasure - With regards to the request to making this an ENUM and updating description to specify values will be in uppercase, the DSB proposes no change as the DSB is not accountable for the taxonomy and will defer to AEMO specifications. The DSB will correct the type to ExternalRef.

These changes have been staged for review: https://github.com/ConsumerDataStandardsAustralia/standards-staging/commit/0cba7f7176bf2943d0457a1bef88496fc78c555f

CDR-API-Stream commented 2 years ago

Issue https://github.com/ConsumerDataStandardsAustralia/standards-maintenance/issues/493 has been staged for review: https://github.com/ConsumerDataStandardsAustralia/standards-maintenance/issues/493#issuecomment-1186741478

CDR-API-Stream commented 2 years ago

To improve consistency, bold all Requirement Level keywords usages (e.g. MUST, SHOULD, ...) throughout the standards.

These changes have been staged for review here: https://github.com/ConsumerDataStandardsAustralia/standards-staging/compare/maintenance/511...maintenance/511-requirement-levels.

Given the large number of changes they have been staged under a separate feature branch off maintenance/511.

Based on feedback, this change won't be included in release 1.18.0. The feedback has been helpful and further changes to deal with consistency for requirement levels will be progressed in Maintenance Iteration 12.