Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

table C code 207yyy #60

Open
vsouvan opened this issue Apr 19, 2020 · 9 comments
Open

table C code 207yyy #60

vsouvan opened this issue Apr 19, 2020 · 9 comments

Comments

@vsouvan
Copy link
Collaborator

vsouvan commented Apr 19, 2020

When code 207yyy appears in a message, it is not properly handled, at least for the bit width part.

The width remains unchanged, but it should be updated as indicated in the WMO BUFR manual.


Imported from Launchpad using lp2gh.

@vsouvan
Copy link
Collaborator Author

vsouvan commented Apr 19, 2020

(by vanh-souvanlasy)
This is the BUFR message at origin of this Bug report which uses the Table C operator 207YYY.
The decoder failed to decode this message with the following warnings:
Warning: unsupported Table C operator 207002 in BUFR version 3

A closer look at the BUFR message show that it is coded as BUFR edition version 3
But since the decoder is implemented in such a way that it only apply Table C operators defined for the specified edition version or before. And since 207YYY is a edition 4 operator. The 207YYY operator are considered unsupported and ignored, which keep the decoder from decoding this message properly.
As a test, a slight modification to allow uses of edition 4 operators with BUFR edition 3 messages seems to correct the problem.

@vsouvan
Copy link
Collaborator Author

vsouvan commented Apr 19, 2020

(by chris-beauregard)
That might be an argument for some kind of "strict mode" flag for the decoder. Not sure where would be the most appropriate place for it, though.

Obviously, we wouldn't want a "strict mode" flag to allow lax encoding practices.

@vsouvan
Copy link
Collaborator Author

vsouvan commented Apr 19, 2020

(by vanh-souvanlasy)
changes applied to decoder to allow uses of BUFR edition 4 Table C operators with BUFR edition 3 messages.

@vsouvan
Copy link
Collaborator Author

vsouvan commented Apr 19, 2020

(by vanh-souvanlasy)
Added a new function to set BUFR rules enforcement level: bufr_set_enforcement(level)
3 possible levels: BUFR_LAX BUFR_WARN_ALLOW BUFR_STRICT
default will be BUFR_WARN_ALLOW
where only BUFR_STRICT will prevent decoder from applying illegal use of Table C operators or some other rules TBD
The program bufr_decoder now has 2 new options "-lax" and "-strict"

@vsouvan
Copy link
Collaborator Author

vsouvan commented Apr 19, 2020

(by chris-beauregard)
Can't say I'm keen on having the global flag and flipping the entire library into a lax/strict mode. I wonder if it can be a property of the message or dataset instead?

@vsouvan
Copy link
Collaborator Author

vsouvan commented Apr 19, 2020

(by vanh-souvanlasy)
The other way would be changing the interface of bufr_decode_message() by add a new parameter to call (strict_level)
would that be better?

@vsouvan
Copy link
Collaborator Author

vsouvan commented Apr 19, 2020

(by chris-beauregard)
That would be the most elegant.

However, I'm not keen on changing the bufr_decode_message() function signature to add a param (breaks source compatibility).

I'd create a new function, perhaps bufr_decode_message_enforced() (ugh; that's horrible, but I can't think of a better name off-hand) with the new strictness parameter, and change bufr_decode_message() to just call the new one with the same semantics as it has now (i.e. BUFR_LAX) would be the least disruptive.

@vsouvan
Copy link
Collaborator Author

vsouvan commented Apr 19, 2020

(by chris-beauregard)
My feeling, though, is that it would be least invasive to have a bufr_set_enforcement() flip a flag in the BUFR_Message structure, so someone could do:

rtrn = bufr_read_message( fp, &msg );
bufr_set_enforcement( msg, BUFR_LAX );
dts = bufr_decode_message( msg, tables );

which can be done simply by adding a function and a flag in the message structure.

@vsouvan
Copy link
Collaborator Author

vsouvan commented Apr 19, 2020

(by chris-beauregard)
Yes, r261 is pretty much perfect. Only one public API call changed, and it's pretty deep. Unlikely anyone was using it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant