ECCC-MSC / libecbufr

libecbufr is a general purpose, template-oriented BUFR encoding/decoding library
Other
10 stars 7 forks source link

table C code 207yyy #60

Open vsouvan opened 4 years ago

vsouvan commented 4 years ago

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 commented 4 years ago

(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 commented 4 years ago

(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 commented 4 years ago

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

vsouvan commented 4 years ago

(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 commented 4 years ago

(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 commented 4 years ago

(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 commented 4 years ago

(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 commented 4 years ago

(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 commented 4 years ago

(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.