Entity-Manager's Configuration Generation leaves address as string

[James Feist]

On 8/28/19 2:38 PM, James Feist wrote:
> On 8/28/19 2:36 PM, Patrick Venture wrote:
>> On Wed, Aug 28, 2019 at 2:29 PM James Feist 
>> <james.feist at linux.intel.com> wrote:
>>>
>>> On 8/28/19 2:16 PM, Patrick Venture wrote:
>>>> On Wed, Aug 28, 2019 at 9:29 AM James Feist 
>>>> <james.feist at linux.intel.com> wrote:
>>>>>
>>>>> On 8/28/19 7:27 AM, Patrick Venture wrote:
>>>>>> I think I've figured out what's happening.
>>>>>>
>>>>>> If a configuration has no fields that are changed by the template 
>>>>>> code
>>>>>> (or possibly even in that case), nothing happens to the values.  So,
>>>>>> the property Address is left "0x54" if that's what it is.  And the
>>>>>> code is templated, so it just adds that property of type string to 
>>>>>> the
>>>>>> dbus sensor configuration.  As this is definitely what I'm seeing.
>>>>>> Json doesn't support ints that are written raw as hex, so wrapping
>>>>>> them as strings is what's required to make the json parse.  I've
>>>>>> worked around this problem by just using decimal everywhere, but
>>>>>> that's harder to read when comparing to schematics.
>>>>>
>>>>> Based on this, I think this line might be your issue:
>>>>>
>>>>> https://github.com/openbmc/entity-manager/blob/3b80d7c51ff5d5859c0ca1f2b517c18f4766a1a6/src/EntityManager.cpp#L1336 
>>>>>
>>>>>
>>>>>
>>>>> If found device is nullopt, you still want to call this line, but you
>>>>> want to call it with an empty flat_map.
>>>>>
>>>>> I verified if this happens it should work here:
>>>>>
>>>>> https://gerrit.openbmc-project.xyz/c/openbmc/entity-manager/+/24787
>>>>
>>>> So, we're still hitting the error:
>>>>
>>>> ...
>>>>       "Exposes": [
>>>>           {
>>>>              "Address": "0x54",
>>>>              "Bus": 31,
>>>>              "Name": "i2cool 0",
>>>>              "Type": "MAX31725"
>>>>           },
>>>> ....
>>>
>>> Did you change the above line? What have you tried? Guessing it has to
>>> do with templateCharReplace not getting hit, if it gets hit it should be
>>> replacing it.
>>
>> Yeah, I see your new unit-test validates the replacement behavior.  So
>> it must be that it's not hitting that line you pointed to.  I'll see
>> if I can grab a test platform tomorrow and see if I can replicate it
>> (we have a config that consistently fails).  And see if I can figure
>> out why the device isn't being "found."
> 
> This would make sense if you had a probe that is set to "TRUE". We 
> normally probe for everything, so this would make sense why we haven't 
> seen it.

This should fix that issue, if it is your issue: 
https://gerrit.openbmc-project.xyz/c/openbmc/entity-manager/+/24796

> 
>>
>>>
>>>>
>>>> Aug 15 22:38:58 MACHINE hwmontempsensor[2697]: terminate called after
>>>> throwing an instance of 'std::bad_variant_access'
>>>>
>>>> It's failing because the configuration is a string "0x54" in dbus.
>>>>
>>>>>
>>>>>
>>>>>
>>>>>>
>>>>>> Would it make sense to make the add property code less field agnostic
>>>>>> so that if the field is Address and the Interface for
>>>>>> configuration.XXX that it checks to see if it's a hex string?  Or,
>>>>>> maybe the templateChar replace -- if that supports converting the hex
>>>>>> string to a raw integer value type should always get hit?
>>
>>
>>
>>>>>>
>>>>>> Thanks,
>>>>>> Patrick
>>>>>>