Skip to content
This repository has been archived by the owner on May 16, 2023. It is now read-only.

Implementation: reference ICAO conversion, with addition of cyrillic and arabic #290

Conversation

janhoffmann
Copy link
Contributor

This PR contains:

  • reference implementation of ICAO conversion algorithm.
  • new: added cyrillic and arabic translations as discussed in Implementation: reference ICAO conversion #284
  • change detection on the name input fields to update std name
  • prefill the standardized name when reading the QR-Code

see #185

The code implements a change detection on the name input fields and prefills the standardized name when reading the QR-Code

see corona-warn-app#185
@janhoffmann janhoffmann requested a review from a team February 4, 2022 12:41
@vaubaehn
Copy link

vaubaehn commented Feb 4, 2022

@janhoffmann @ggrund-tsi
First of all thanks a lot to @janhoffmann for your effort!
But I would suggest to wait a bit before merging this PR to have Ubirch have a look on it - as Ubirch has kind of a reference implementation for the vaccination DCCs.
I'll add more information in an hour, as I need to be off from keyboard now.
Thanks for your patience.

@vaubaehn
Copy link

vaubaehn commented Feb 4, 2022

Hi @thinkberg ,
I'm pinging you from a PR that is about to implement ICAO transliteration into TSI's Schnelltestportal, thanks to the great effort of @janhoffmann !
We talked about heterogenity of ICAO transliteration quality, resulting in errors to match DCCs to the same person, in this issue: #185 (comment).
We are aware that not only ICAO transliteration is a source of error, but also data sources themselves.
However, the PR of @janhoffmann is not only the chance to shift away from manual entry of standardized names into TSI's Schnelltestportal, but this implementation may also serve as a reference implementation for TSI's backend API partners in the future. @ggrund-tsi from TSI already signaled to accept a PR for the implementation into the RAT portal in principle (#284 (comment)).

As Ubirch already has an implementation for ICAO transliteration for the vaccination DCCs, it would be the biggest gain, if the here proposed PR is aligned as best as possible to your implementation. Therefore, I kindly ask you to have a review on the here proposed solution in terms of completeness and conformity to the ICAO algorithm.
Would it be possible that one of your devs has a look here?

In any case, please leave a short line, whether a review from your side is possible.

Thank you very much in advance!

/cc: @mlenkeit @thomasaugsten @oliver-steinbrecher
/FYI: @timokoenig @Ein-Tim @dsarkar

@janhoffmann janhoffmann changed the title Implementation: reference ICAE conversion, with addition of cyrillic and arabic Implementation: reference ICAO conversion, with addition of cyrillic and arabic Feb 4, 2022
@vaubaehn
Copy link

vaubaehn commented Mar 3, 2022

Hi all,
I again asked @thinkberg for support to align the here proposed ICAO algorithm to Ubirch's, or to open source Ubirch's algorithm in the "certification apis repository":
Digitaler-Impfnachweis/certification-apis#232

@vaubaehn
Copy link

vaubaehn commented Mar 8, 2022

Hi all again,
as mentioned in my last comment, I asked Ubirch for support with regard to their ICAO implemention (Digitaler-Impfnachweis/certification-apis#232).
The response is, that it is planned to open source their ICAO implementation, but we do not know when this will be the case.
I therefore vote, that @janhoffmann 's PR should be merged as soon as possible, and a comprehensive review regarding the very details of the ICAO algorithm (like trasliteration of letters like "A ring above -> AA or A?") could be done after the public release of Ubirch's algorithm.

To enable @janhoffmann to correct some functional problems with the proposed PR, he asked whether it's possible to get him a working enviroment for testing.
Is this something you could support him @ggrund-tsi or @BrittaTSI ?
Thanks a lot!

@janhoffmann
Copy link
Contributor Author

janhoffmann commented Mar 17, 2022

@vaubaehn this here is really frustrating.

We have a working (afaik) solution which would improve the work of all stations immediately. Even if it's not matching Ulbrichts implentation this is not a good way to handle this. If we have a difference in some implementation details it will probably only affect a very small percentage of cases.
Right now we have a way higher error source: humans.

I already saw stations hacking chrome plugins to bypass this long lasting problem.

I offered my help on July 2021 and got hardly any feedback for months. After the hint to the DCC implementation guide we did this work in 1 day with a second revision within 1 extra day.

Is there anyone at T-Systems, Ulbricht, SAP caring for this?!
Is there any way to speed up the process?

@vaubaehn
Copy link

@vaubaehn this here is really frustrating.

@janhoffmann I fully agree. Insufficient communication from the stakeholders towards the community let us feel ignored, and valuable volunteer contributions sometimes seem to rot in issues and open PRs. I observe that some parts of the most active community is losing their motivation to further continue participating in this project (me included).
At least a better communication and the will to look into and to deal with community contributions more deeply could solve some of the problems currently experienced.

Is there anyone at T-Systems, Ulbricht, SAP caring for this?!

It's up to TSI solely. SAP can't do much, as the issue handled here is not in their scope - except raising this issue in their meetings with TSI. Ubirch could open source their ICAO implementation, but they're quite busy with other things like the DCC re-issuing API currently. And their ICAO implementation would help, but getting the RAT portal enhanced with your PR has definetly higher priority, imho - particularly as incidence rates are rising again, and more tests are conducted.

Is there any way to speed up the process?

In many cases stakeholders see the responsibility at RKI/BMG, for example with the overcrowded and in some cases low quality TAN hotline (corona-warn-app/cwa-hotline#14), even call centers are contracted by TSI. In my personal opinion, some quality assurance should be undertaken from TSI, as all these problems will fall back to TSI and damage their reputation (despite RKI would need to fund capacities).

We can only guess, why the PR here is ignored. Whether RKI is not funding enough to pay one TSI dev to have a look into, whether RKI is not funding enough to have enough staff at TSI to be more flexible to maintain their services, or whether there is just no interest and the community is seen as stupid nerds.

The only thing I can think about to speed things up is to make problems visible to the public, e.g. contacting public media. Unfortunately, reports published in media like ComputerBild or Chip/Focus Online have more power to damage the project's reputation than helping to get things done.

I'm sorry, I think I can't and won't do more anymore.

@vaubaehn
Copy link

Just one last word to TSI:
If you contributed volunteerly to such a project - after the course of so many issues here, wouldn't you feel treated respectless?

@vaubaehn
Copy link

Hi @timokoenig ,
unfortunately we're not in the maintainers group, so we're blind with regard to the docs 😉
Anyway, TSI could look into @janhoffmann 's PR, re-fining the ICAO algorithm regarding the handling of some special letter transliterations could be done later, imho.
But like written above, I'm out - for me, the time that I invested as a volunteer is just treated respectless in my pov, so I will shift to other things in life.
Thanks for all your great support in all these issues, Timo! ❤️

@timokoenig
Copy link

I wasn't event aware that this was a private repository. I removed the links because even I just got removed from that group. At least someone is watching this PR silently 😄

Sad to hear but I can understand your point. Thank you too for all the support!

@Ein-Tim
Copy link
Contributor

Ein-Tim commented Mar 17, 2022

I'm sorry, I think I can't and won't do more anymore.

Hopefully this is not a Goodbye but only a see you soon! 😢

@vaubaehn Thanks for your continuous support, we (the community) really appreciate it!

@thinkberg
Copy link

We have this planned, but as @vaubaehn explained we are a bit under right now.

@thinkberg
Copy link

I will try to at least get the docs out as we did take the comments into account and improved the conversion. I will check over the weekend.

@vaubaehn
Copy link

@thinkberg

I will try to at least get the docs out as we did take the comments into account and improved the conversion. I will check over the weekend.

Thank you!

@vaubaehn
Copy link

@Ein-Tim

Hopefully this is not a Goodbye but only a see you soon! 😢

For this repo, I'm finished. I will do some housekeeping in other repos, await CWA 2.19.1, and after that there will be a proper good-bye over at CWA 😉

@Ein-Tim
Copy link
Contributor

Ein-Tim commented Mar 24, 2022

FYI: https://github.com/corona-warn-app/cwa-icao-transliteration

No idea if this helps here.

@janhoffmann
Copy link
Contributor Author

@Ein-Tim great so see progress, but this only shows why nobody should make a effort to contribute here.

The implementation of @ggrund-tsi is the tsi way of saying "screw the open source community, we code it ourself". I'm not sad because my work will never be used, I'm sad because it was ignored during the development of their own solution.
I'm curious how long it will take to see this chances in the real world software: my guess, may 2022.

To sum up: next time I will code my own interface and register as a API partner, this is a joke.

@fOppenheimer
Copy link
Contributor

Sorry, Jan. On Friday last week I decided to use another approach and it worked for us. Thank you very much for your ideas, we mixed them with ours.

@Ein-Tim
Copy link
Contributor

Ein-Tim commented Mar 25, 2022

FYI: #296

@f11h
Copy link
Member

f11h commented Jan 23, 2023

Already implemented in #296

@f11h f11h closed this Jan 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants