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

optimizer incorrectly handles report count #7

Open
ajmirsky opened this issue Jan 3, 2023 · 3 comments
Open

optimizer incorrectly handles report count #7

ajmirsky opened this issue Jan 3, 2023 · 3 comments

Comments

@ajmirsky
Copy link

ajmirsky commented Jan 3, 2023

>> cat test.py
from hrdc.usage import *
from hrdc.descriptor import *

descriptor = Collection(Collection.Application, sensors.Environmental,
    Collection(Collection.Physical, sensors.Environmental,
        Report(1,
            Value(Value.Input, sensors.EnvironmentalHumidity, 16, logicalMin = 0, logicalMax = 1400),
            Value(Value.Input, sensors.EnvironmentalTemperature, 8, logicalMin = 0, logicalMax = 100, count = 2),
            Value(Value.Input, sensors.EnvironmentalAtmosphericPressure, 8, logicalMin = 0, logicalMax = 100),
        ),
    ),
)
compile_main(descriptor)

>> python test.py --output=hex

generates

05 20
09 30
a1 01
09 30
a1 00
85 01
15 00
26 78 05
75 10
95 01
09 32
81 02
09 33
25 64
75 08    
             << Value Input is missing a report count, it should have a report count = 2 (95 02)
09 31
95 03    << report count is 3, but should be report count = 1 (95 01)
81 02
c0
c0
@nipo
Copy link
Owner

nipo commented Jan 3, 2023

Current output is easier to grok with comments, like in:

$ python3 tests/issue_7/test.py -o code
     0x05, 0x20,                    // UsagePage (sensors)
     0x09, 0x30,                    // Usage (Environmental)
     0xa1, 0x01,                    // Collection (Application)
     0x09, 0x30,                    //     Usage (Environmental)
     0xa1, 0x00,                    //     Collection (Physical)
     0x85, 0x01,                    //         ReportID (1)
     0x15, 0x00,                    //         LogicalMinimum (0)
     0x26, 0x78, 0x05,              //         LogicalMaximum (1400)
     0x75, 0x10,                    //         ReportSize (16)
     0x95, 0x01,                    //         ReportCount (1)
     0x09, 0x32,                    //         Usage (EnvironmentalHumidity)
     0x81, 0x02,                    //         Input (Variable)
 
    // Part of interest
     0x09, 0x33,                    //         Usage (EnvironmentalTemperature)
     0x25, 0x64,                    //         LogicalMaximum (100)
     0x75, 0x08,                    //         ReportSize (8)
     0x09, 0x31,                    //         Usage (EnvironmentalAtmosphericPressure)
     0x95, 0x03,                    //         ReportCount (3)
     0x81, 0x02,                    //         Input (Variable)

     0xc0,                          //     EndCollection
     0xc0,                          // EndCollection

Part of interest is broken. We should either have:

     0x09, 0x33,                    //         Usage (EnvironmentalTemperature)
     0x25, 0x64,                    //         LogicalMaximum (100)
     0x75, 0x08,                    //         ReportSize (8)
     0x95, 0x02,                    //         ReportCount (2)
     0x81, 0x02,                    //         Input (Variable)
     0x09, 0x31,                    //         Usage (EnvironmentalAtmosphericPressure)
     0x95, 0x01,                    //         ReportCount (1)
     0x81, 0x02,                    //         Input (Variable)

Or have:

     0x09, 0x33,                    //         Usage (EnvironmentalTemperature)
     0x09, 0x33,                    //         Usage (EnvironmentalTemperature)
     0x09, 0x31,                    //         Usage (EnvironmentalAtmosphericPressure)
     0x25, 0x64,                    //         LogicalMaximum (100)
     0x75, 0x08,                    //         ReportSize (8)
     0x95, 0x03,                    //         ReportCount (3)
     0x81, 0x02,                    //         Input (Variable)

I have a bias for the latter.

nipo added a commit that referenced this issue Jan 3, 2023
This is kind of a quick fix for #7, actual fix would be to monitor stream for
usages and duplicate array items only if single usage value is used for more
than one item.
@nipo
Copy link
Owner

nipo commented Jan 3, 2023

I committed ad4dd13 to get a semantically-correct output stream. I would prefer the second output, though.

@ajmirsky
Copy link
Author

ajmirsky commented Jan 4, 2023

thanks for the quick fix!

(great library by the way!)

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

No branches or pull requests

2 participants