[Question] How can we use underscore "_" in sensor name?

[Question] How can we use underscore "_" in sensor name?

[Heyi Guo]

Hi all,

Can we use underscore "_" in IPMI sensor names? Now we see that "_" in 
sensor names will be replaced with space in the code, even if we really 
use "_" for sensor names in json configuration files.

The background is that we used underscore "_" in sensor names in legacy 
BMC, and all the OOB software uses such names to get sensor data. 
Therefore we want to make it compatible for switching to OpenBMC.

Is there any way to achieve this?

Thanks,

Heyi

Re: [Question] How can we use underscore "_" in sensor name?

[Johnathan Mantey]

[-- Attachment #1.1: Type: text/plain, Size: 1263 bytes --]

Heyi,

On 9/13/21 3:49 AM, Heyi Guo wrote:
> Hi all,
> 
> Can we use underscore "_" in IPMI sensor names? Now we see that "_" in 
> sensor names will be replaced with space in the code, even if we really 
> use "_" for sensor names in json configuration files.
> 

Which layer is translating the "_" char?
Are you referring to D-Bus?
IPMITool?
Some other locatioN?

It would be helpful to know.
As an example, D-Bus translates "-" to "%2D" and back because D-Bus, or 
one of the D-Bus consumers can't accept the hyphen character.

It is likely that the underscore character has some similar naming 
convention issue.

You may want to investigate using the same scheme to translate between 
"_" and a "%5F" combination.

> The background is that we used underscore "_" in sensor names in legacy 
> BMC, and all the OOB software uses such names to get sensor data. 
> Therefore we want to make it compatible for switching to OpenBMC.
> 
> Is there any way to achieve this?
> 
> Thanks,
> 
> Heyi
> 

-- 
Johnathan Mantey
Senior Software Engineer
*azad te**chnology partners*
Contributing to Technology Innovation since 1992
Phone: (503) 712-6764
Email: johnathanx.mantey@intel.com <mailto:johnathanx.mantey@intel.com>



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

Re: [Question] How can we use underscore "_" in sensor name?

[Phil Eichinger]

On 2021-09-13 12:49, Heyi Guo wrote:
> Hi all,
> 
> Can we use underscore "_" in IPMI sensor names? Now we see that "_" in
> sensor names will be replaced with space in the code, even if we
> really use "_" for sensor names in json configuration files.
> 
> The background is that we used underscore "_" in sensor names in
> legacy BMC, and all the OOB software uses such names to get sensor
> data. Therefore we want to make it compatible for switching to
> OpenBMC.
> 
> Is there any way to achieve this?

This works here, using phosphor-hwmon, it does not replace underscores 
in sensor names.
DBus should allow this: "Each element must only contain the ASCII 
characters "[A-Z][a-z][0-9]_"
phosphor-webui however does replace underscores with spaces.

Cheers
Phil

Re: [Question] How can we use underscore "_" in sensor name?

[Heyi Guo]

Hi Johnathan,

The code in dbus-sensors will translate sensor name by replacing space " 
" to "_", e.g. ADCSensor.cpp:

53     Sensor(boost::replace_all_copy(sensorName, " ", "_"),

Then in phosphor-ipmi-host dbus-sdr/sensorcommands.cpp, it will try to 
revert "_" back to " ":

  419     std::replace(name.begin(), name.end(), '_', ' ');

The first conversion does nothing for our case, but the 2nd conversion 
will unconditionally convert "_" to space " ".

If we don't really forbid to use "_" in sensor names, I think we'd 
better replace space " " with %20 in dbus-sensors, and convert %20 back 
to " " in ipmi-host.

Thanks,

Heyi

On 2021/9/13 下午10:57, Johnathan Mantey wrote:
> Heyi,
>
> On 9/13/21 3:49 AM, Heyi Guo wrote:
>> Hi all,
>>
>> Can we use underscore "_" in IPMI sensor names? Now we see that "_" 
>> in sensor names will be replaced with space in the code, even if we 
>> really use "_" for sensor names in json configuration files.
>>
>
> Which layer is translating the "_" char?
> Are you referring to D-Bus?
> IPMITool?
> Some other locatioN?
>
> It would be helpful to know.
> As an example, D-Bus translates "-" to "%2D" and back because D-Bus, 
> or one of the D-Bus consumers can't accept the hyphen character.
>
> It is likely that the underscore character has some similar naming 
> convention issue.
>
> You may want to investigate using the same scheme to translate between 
> "_" and a "%5F" combination.
>
>> The background is that we used underscore "_" in sensor names in 
>> legacy BMC, and all the OOB software uses such names to get sensor 
>> data. Therefore we want to make it compatible for switching to OpenBMC.
>>
>> Is there any way to achieve this?
>>
>> Thanks,
>>
>> Heyi
>>
>

Re: [Question] How can we use underscore "_" in sensor name?

[Oskar Senft]

+1 to using something more explicit, like %20 instead of "_" for
replacing " " for D-bus names.

We're facing the same problem where "_" in sensor names from D-bus get
unconditionally replaced with " " in dbus-sensors. This is currently
preventing us from "converting" an older platform to Entity Manager.

Oskar.

On Wed, Sep 15, 2021 at 12:58 AM Heyi Guo <guoheyi@linux.alibaba.com> wrote:
>
> Hi Johnathan,
>
> The code in dbus-sensors will translate sensor name by replacing space "
> " to "_", e.g. ADCSensor.cpp:
>
> 53     Sensor(boost::replace_all_copy(sensorName, " ", "_"),
>
> Then in phosphor-ipmi-host dbus-sdr/sensorcommands.cpp, it will try to
> revert "_" back to " ":
>
>   419     std::replace(name.begin(), name.end(), '_', ' ');
>
> The first conversion does nothing for our case, but the 2nd conversion
> will unconditionally convert "_" to space " ".
>
> If we don't really forbid to use "_" in sensor names, I think we'd
> better replace space " " with %20 in dbus-sensors, and convert %20 back
> to " " in ipmi-host.
>
> Thanks,
>
> Heyi
>
> On 2021/9/13 下午10:57, Johnathan Mantey wrote:
> > Heyi,
> >
> > On 9/13/21 3:49 AM, Heyi Guo wrote:
> >> Hi all,
> >>
> >> Can we use underscore "_" in IPMI sensor names? Now we see that "_"
> >> in sensor names will be replaced with space in the code, even if we
> >> really use "_" for sensor names in json configuration files.
> >>
> >
> > Which layer is translating the "_" char?
> > Are you referring to D-Bus?
> > IPMITool?
> > Some other locatioN?
> >
> > It would be helpful to know.
> > As an example, D-Bus translates "-" to "%2D" and back because D-Bus,
> > or one of the D-Bus consumers can't accept the hyphen character.
> >
> > It is likely that the underscore character has some similar naming
> > convention issue.
> >
> > You may want to investigate using the same scheme to translate between
> > "_" and a "%5F" combination.
> >
> >> The background is that we used underscore "_" in sensor names in
> >> legacy BMC, and all the OOB software uses such names to get sensor
> >> data. Therefore we want to make it compatible for switching to OpenBMC.
> >>
> >> Is there any way to achieve this?
> >>
> >> Thanks,
> >>
> >> Heyi
> >>
> >

Re: [Question] How can we use underscore "_" in sensor name?

[Johnathan Mantey]

[-- Attachment #1.1: Type: text/plain, Size: 1436 bytes --]

On 9/14/21 9:57 PM, Heyi Guo wrote:
> Hi Johnathan,
> 
> The code in dbus-sensors will translate sensor name by replacing space " 
> " to "_", e.g. ADCSensor.cpp:
> 
> 53     Sensor(boost::replace_all_copy(sensorName, " ", "_"),
> 
> Then in phosphor-ipmi-host dbus-sdr/sensorcommands.cpp, it will try to 
> revert "_" back to " ":
> 
>   419     std::replace(name.begin(), name.end(), '_', ' ');
> 
> The first conversion does nothing for our case, but the 2nd conversion 
> will unconditionally convert "_" to space " ".
> 
> If we don't really forbid to use "_" in sensor names, I think we'd 
> better replace space " " with %20 in dbus-sensors, and convert %20 back 
> to " " in ipmi-host.
> 

Rereading your original post you were having issues with sensor names 
you explicitly had "_" characters, and the line on 419 turns it into a 
blank space. The underscore character is overloaded at the D-Bus level.

I expect there may be an issue with my proposed solution, as D-Bus is 
probably not going to allow the use of the '%' character. A solution 
that fits within the D-Bus naming rules will be required to provide a 
reliable encode/decode scheme.

<snip>

-- 
Johnathan Mantey
Senior Software Engineer
*azad te**chnology partners*
Contributing to Technology Innovation since 1992
Phone: (503) 712-6764
Email: johnathanx.mantey@intel.com <mailto:johnathanx.mantey@intel.com>



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

Re: [Question] How can we use underscore "_" in sensor name?

[Patrick Williams]

[-- Attachment #1: Type: text/plain, Size: 767 bytes --]

On Wed, Sep 15, 2021 at 09:54:47AM -0400, Oskar Senft wrote:
> +1 to using something more explicit, like %20 instead of "_" for
> replacing " " for D-bus names.

Ed had previously added code to sdbusplus to consistently do conversions like
this, in the same way that systemd tends to do conversions.  I think we just
need to convert the affected repositories here to use these sdbusplus
object_path types rather than creating their own strings.

https://github.com/openbmc/sdbusplus/blob/master/src/message/native_types.cpp#L53

If paths are encoded this way, we are able to clearly differentiate between a
desired space and an underscore because the spaces become encoded as something
like `_20`, similar to the proposal here.

-- 
Patrick Williams

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [Question] How can we use underscore "_" in sensor name?

[Ed Tanous]

On Wed, Sep 15, 2021 at 9:23 AM Patrick Williams <patrick@stwcx.xyz> wrote:
>
> On Wed, Sep 15, 2021 at 09:54:47AM -0400, Oskar Senft wrote:
> > +1 to using something more explicit, like %20 instead of "_" for
> > replacing " " for D-bus names.
>
> Ed had previously added code to sdbusplus to consistently do conversions like
> this, in the same way that systemd tends to do conversions.  I think we just
> need to convert the affected repositories here to use these sdbusplus
> object_path types rather than creating their own strings.
>
> https://github.com/openbmc/sdbusplus/blob/master/src/message/native_types.cpp#L53
>
> If paths are encoded this way, we are able to clearly differentiate between a
> desired space and an underscore because the spaces become encoded as something
> like `_20`, similar to the proposal here.

For what it's worth, I added this code to solve this exact problem
(although in my case I wanted to use dashes in sensor names) I just
haven't pushed the final sets of patches yet to turn it on.  I'm glad
to see all the interest in this;  I think the bulk of the hard work is
done, and all we should need is changesets pushed to bmcweb,
dbus-sensors, and ipmi-dynamic to use the operator/ and filename()
methods to construct the dbus paths.  Keep in mind, IPMI will need
some escaping, as IPMI SDRs assume ASCII, and bmcweb will need to be
flushed through the urlencode method to ensure we're encoding our URIs
properly.

If this is important to people, feel free to push and test the
patches, otherwise it's still on my list to get done and I'll CC this
thread when any new patchsets are pushed.

>
> --
> Patrick Williams

Re: [Question] How can we use underscore "_" in sensor name?

[Heyi Guo]

Hi Ed,

That's great, and I'll be glad to test the patches on our platforms.

Thanks,

Heyi

On 2021/9/16 上午12:35, Ed Tanous wrote:
> On Wed, Sep 15, 2021 at 9:23 AM Patrick Williams <patrick@stwcx.xyz> wrote:
>> On Wed, Sep 15, 2021 at 09:54:47AM -0400, Oskar Senft wrote:
>>> +1 to using something more explicit, like %20 instead of "_" for
>>> replacing " " for D-bus names.
>> Ed had previously added code to sdbusplus to consistently do conversions like
>> this, in the same way that systemd tends to do conversions.  I think we just
>> need to convert the affected repositories here to use these sdbusplus
>> object_path types rather than creating their own strings.
>>
>> https://github.com/openbmc/sdbusplus/blob/master/src/message/native_types.cpp#L53
>>
>> If paths are encoded this way, we are able to clearly differentiate between a
>> desired space and an underscore because the spaces become encoded as something
>> like `_20`, similar to the proposal here.
> For what it's worth, I added this code to solve this exact problem
> (although in my case I wanted to use dashes in sensor names) I just
> haven't pushed the final sets of patches yet to turn it on.  I'm glad
> to see all the interest in this;  I think the bulk of the hard work is
> done, and all we should need is changesets pushed to bmcweb,
> dbus-sensors, and ipmi-dynamic to use the operator/ and filename()
> methods to construct the dbus paths.  Keep in mind, IPMI will need
> some escaping, as IPMI SDRs assume ASCII, and bmcweb will need to be
> flushed through the urlencode method to ensure we're encoding our URIs
> properly.
>
> If this is important to people, feel free to push and test the
> patches, otherwise it's still on my list to get done and I'll CC this
> thread when any new patchsets are pushed.
>
>> --
>> Patrick Williams