* [Question] How can we use underscore "_" in sensor name? @ 2021-09-13 10:49 Heyi Guo 2021-09-13 14:57 ` Johnathan Mantey 2021-09-13 15:22 ` Phil Eichinger 0 siblings, 2 replies; 9+ messages in thread From: Heyi Guo @ 2021-09-13 10:49 UTC (permalink / raw) To: Vernon Mauery, Tom Joseph, Puli, Apparao; +Cc: openbmc 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Question] How can we use underscore "_" in sensor name? 2021-09-13 10:49 [Question] How can we use underscore "_" in sensor name? Heyi Guo @ 2021-09-13 14:57 ` Johnathan Mantey 2021-09-15 4:57 ` Heyi Guo 2021-09-13 15:22 ` Phil Eichinger 1 sibling, 1 reply; 9+ messages in thread From: Johnathan Mantey @ 2021-09-13 14:57 UTC (permalink / raw) To: Heyi Guo, Vernon Mauery, Tom Joseph, Puli, Apparao; +Cc: openbmc [-- 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 --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Question] How can we use underscore "_" in sensor name? 2021-09-13 14:57 ` Johnathan Mantey @ 2021-09-15 4:57 ` Heyi Guo 2021-09-15 13:54 ` Oskar Senft 2021-09-15 14:36 ` Johnathan Mantey 0 siblings, 2 replies; 9+ messages in thread From: Heyi Guo @ 2021-09-15 4:57 UTC (permalink / raw) To: Johnathan Mantey, Vernon Mauery, Tom Joseph, Puli, Apparao; +Cc: openbmc 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 >> > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Question] How can we use underscore "_" in sensor name? 2021-09-15 4:57 ` Heyi Guo @ 2021-09-15 13:54 ` Oskar Senft 2021-09-15 16:23 ` Patrick Williams 2021-09-15 14:36 ` Johnathan Mantey 1 sibling, 1 reply; 9+ messages in thread From: Oskar Senft @ 2021-09-15 13:54 UTC (permalink / raw) To: Heyi Guo, openbmc Cc: Vernon Mauery, Tom Joseph, Puli, Apparao, Johnathan Mantey +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 > >> > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Question] How can we use underscore "_" in sensor name? 2021-09-15 13:54 ` Oskar Senft @ 2021-09-15 16:23 ` Patrick Williams 2021-09-15 16:35 ` Ed Tanous 0 siblings, 1 reply; 9+ messages in thread From: Patrick Williams @ 2021-09-15 16:23 UTC (permalink / raw) To: Oskar Senft Cc: Vernon Mauery, openbmc, Tom Joseph, Heyi Guo, Johnathan Mantey, Puli, Apparao [-- 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 --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Question] How can we use underscore "_" in sensor name? 2021-09-15 16:23 ` Patrick Williams @ 2021-09-15 16:35 ` Ed Tanous 2021-09-18 2:38 ` Heyi Guo 0 siblings, 1 reply; 9+ messages in thread From: Ed Tanous @ 2021-09-15 16:35 UTC (permalink / raw) To: Patrick Williams Cc: Vernon Mauery, openbmc, Tom Joseph, Heyi Guo, Oskar Senft, Johnathan Mantey, Puli, Apparao 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Question] How can we use underscore "_" in sensor name? 2021-09-15 16:35 ` Ed Tanous @ 2021-09-18 2:38 ` Heyi Guo 0 siblings, 0 replies; 9+ messages in thread From: Heyi Guo @ 2021-09-18 2:38 UTC (permalink / raw) To: Ed Tanous, Patrick Williams Cc: Vernon Mauery, openbmc, Tom Joseph, Oskar Senft, Johnathan Mantey, Puli, Apparao 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Question] How can we use underscore "_" in sensor name? 2021-09-15 4:57 ` Heyi Guo 2021-09-15 13:54 ` Oskar Senft @ 2021-09-15 14:36 ` Johnathan Mantey 1 sibling, 0 replies; 9+ messages in thread From: Johnathan Mantey @ 2021-09-15 14:36 UTC (permalink / raw) To: Heyi Guo, Vernon Mauery, Tom Joseph, Puli, Apparao; +Cc: openbmc [-- 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 --] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Question] How can we use underscore "_" in sensor name? 2021-09-13 10:49 [Question] How can we use underscore "_" in sensor name? Heyi Guo 2021-09-13 14:57 ` Johnathan Mantey @ 2021-09-13 15:22 ` Phil Eichinger 1 sibling, 0 replies; 9+ messages in thread From: Phil Eichinger @ 2021-09-13 15:22 UTC (permalink / raw) To: Heyi Guo; +Cc: openbmc 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2021-09-18 2:39 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-09-13 10:49 [Question] How can we use underscore "_" in sensor name? Heyi Guo 2021-09-13 14:57 ` Johnathan Mantey 2021-09-15 4:57 ` Heyi Guo 2021-09-15 13:54 ` Oskar Senft 2021-09-15 16:23 ` Patrick Williams 2021-09-15 16:35 ` Ed Tanous 2021-09-18 2:38 ` Heyi Guo 2021-09-15 14:36 ` Johnathan Mantey 2021-09-13 15:22 ` Phil Eichinger
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).