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

Problems with "domain" values in the objdictedit GUI tool #10

Closed
robybona opened this issue Jan 26, 2024 · 5 comments · Fixed by #16
Closed

Problems with "domain" values in the objdictedit GUI tool #10

robybona opened this issue Jan 26, 2024 · 5 comments · Fixed by #16

Comments

@robybona
Copy link

The objdictedit GUI tool seems to incorrectly manage "domain" values.

How to reproduce:

  • launch the objdictedit GUI tool;
  • File -> New;
  • fill in the "Name:" field then click "OK";
  • click the "0x2000-0x5FFF Manufacturer Specific" entry;
  • click "Add" and add a map variable, Index 0x2000, "VAR" type, give it a name, and click "OK";
  • click the new added variable, change the type to "DOMAIN";
  • the following appears in the "value" field: b'';
  • initialize the variable by changing the "value" field, then execute "Build Dictionary".

Here's the problem, because:

  • if the value b'00', or b'11', or b'25252525' is entered, building the dictionary gives the error: "Domain variable not initialized, index: 0x2000, subindex: 0x00";
  • if the value 12 is entered, when confirming the data the following error appears: <class 'AttributeError'>: 'bytes' object has no attribute 'encode';
  • if the value 0x1234 is entered, when confirming the data the following error appears: <class 'binascii.Error'>: decoding with 'hex_codec' codec failed (Error: Non-hexadecimal digit found).

It seems there's no way to insert an acceptable value for "domain" fields.

Something similar happens when opening a .od file with the objdictedit GUI tool.
Here's an example.
Consider the 0x2300 entry, of "domain" type, with the hexadecimal value: 0x00000000000000. It is 7 bytes long. The .od content is:

  <entry>
    <key type="numeric" value="8960" />
    <val type="string" value="\x00\x00\x00\x00\x00\x00\x00" />
  </entry>

When this .od file is opened by the objdictedit GUI tool, the value field shows: b'5c7830305c7830305c7830305c7830305c7830305c7830305c783030', that's clearly not the expected one.
It should show 00000000000000.
Now, without changing the value, executing "Build dictionary" succeeds. Anyway, the entry is generated like this, in the .c file:

UNS8 myDomainField[28] = "\x5c\x78\x30\x30\x5c\x78\x30\x30\x5c\x78\x30\x30\x5c\x78\x30\x30\x5c\x78\x30\x30\x5c\x78\x30\x30\x5c\x78\x30\x30";		/* Mapped at index 0x2300, subindex 0x00 */

Again, that's not the expected size and value. It should be:

UNS8 myDomainField[7] = "\x00\x00\x00\x00\x00\x00\x00";		/* Mapped at index 0x2300, subindex 0x00 */

Here's the behaviour of older versions of objdictedit (I mean: the ones working with Python 2 and wxPython 2.7).
The "domain" value accepted entries like 0x12fc (it was transformed as 12fc when confirmed), or like 12fc.
The general rule was to accept any entry with an even quantity of hexadecimal characters. The entry could be prefixed by 0x, but it was not mandatory. If something different was entered, like non-hexadecimal characters or an odd quantity of characters, the entry was not accepted and the cell was filled back with the old valid value.

@sveinse
Copy link
Member

sveinse commented Jan 28, 2024

I can confirm the bug. This is py2 -> py3 migration error.

FYI: I'm currently working on a py2 cleanup and with that I believe the XML part will fixed. The UI error still remains.

sveinse added a commit that referenced this issue Apr 10, 2024
* Fix failure on reading the "need" field from node
* Increase the viewport size of the index parts in GUI
* Add error message if entering invalid data in GUI table editor
* Fix bug where UI changes in the table didn't update the data
* Added in-line documentation in ODStructTypes
* Fix incorrect ValueError when having arrays
* FIxed Node.DumpFile not guessing the filetype correctly
* Fix handling of DOMAIN, which address #10
Testing
* Add new "equiv_files" fixture to test equivalent files
* Add test for testing `odg compare`
* Added OD-file for DOMAIN and updated OD-files for consistency
@sveinse
Copy link
Member

sveinse commented Apr 11, 2024

The main branch now includes a fix for the DOMAIN object type. @robybona please check if this fixes the problem for you.

@robybona
Copy link
Author

@sveinse I made some tests.
The GUI editing is way better. Numeric values are shown, even during the editing and the confirmation.
Anyway, an error is shown if "high" values are inserted.

Let's say it's needed to create a 6-bytes domain (without indicating the buffer size).
Writing 000000000000 or 111111111111 all is OK.
Writing 888888888888 or 999999999999 or ABCDEFABCDEF gives:
"Failed to set value"
"Failed to set value: 'utf-8' codec can't decode byte 0xab in position 0: invalid start byte".

Another problem is that the value saved in the .od file is formatted differently, basing on the value itself.
Examples:
000000000000 saved as: <val type="string" value="\x00\x00\x00\x00\x00\x00" />
111111111111 saved as: <val type="string" value="\x11\x11\x11\x11\x11\x11" />
123412341234 saved as: <val type="string" value="\x124\x124\x124" />

All the three values are accepted by the GUI, but the last one is represented in an unexpected way.
The correct representation is given by six groups of two nibbles (a single byte each).
Despite the representation inside the .od file, the .c and .h are correctly generated (the generated array is six bytes); closing and reopening the resulting .od file, the correct value is shown on the GUI.

So, summarizing, the remaining problems are:

  1. it's not possible to insert all the hexadecimal values when editing a domain;
  2. the domain representation in the .od file is not uniform and depends on the inserted value.

@sveinse
Copy link
Member

sveinse commented Apr 13, 2024

@robybona Thank you for the report. I've fixed the problems in PR #16. The values you shows were missing from the test suite and didn't work. This is now fixes. Please retest the branch if you have the opportunity.

May I add that "12341234" is actually correct with "\x124\x124". This can be represented by [0x12, '4', 0x12, '4']. The 4 char is represented with 0x34.

PS! This issue was not trivial to fix. Spent the better part of a day fixing it. This problem touches file encoding which becomes complicated with legacy py2 OD compatibility. The legacy py2 objdictgen OD-format is ill-suited for how py3 handles strings and unicode. Since py3 doesn't have unicode and strings, DOMAIN broke in py3.

Symbols like \x00 and \xAA is not encodable in utf-8 -- it's rather the job for bytes. Especially >0x80 which interferes with utf-8 multi-byte handling. This we ought to avoid using string for representing DOMAIN. However due to compatibility with the legacy py2 objdictgen format I had to find a way. Py2 doesn't have bytes so we can't use that if the OD files shall remain compatible with py2 objdictgen.

Well, now I really hope it is done.

@robybona
Copy link
Author

@sveinse I tested the fix-domain-handling branch, #16, and it seems that everything is OK now.

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

Successfully merging a pull request may close this issue.

2 participants