There is already an sdbusplus loop running, it's in Mainloop::run(). [Thu] I think phosphor-hwmon is using the timer for Mainloop::run() which triggers each 1 seconds. Is this right? To support signal the changing in dbus property, we need a service which listens the dbus signal, right? I'm still fresh in C++ and dbus code. So please correct me if I'm wrong. Thanks. Thu Nguyen. On Fri, Nov 6, 2020 at 3:52 AM Matt Spinler wrote: > > > On 11/4/2020 4:18 PM, Thu Ba Nguyen wrote: > > Please see my comments below. Thanks. Thu Nguyen. On Wed, Nov 4, 2020 > > at 11:31 PM... > > This Message Is From an External Sender > > This message came from outside your organization. > > > > Please see my comments below. > > > > Thanks. > > Thu Nguyen. > > > > On Wed, Nov 4, 2020 at 11:31 PM Matt Spinler > > wrote: > > > > > > > > On 11/4/2020 3:15 AM, Thu Ba Nguyen wrote: > > > Hi Matt, I implemented "Inactive the host sensors" in > > phosphor-hwmon > > > use below... > > > This Message Is From an External Sender > > > This message came from outside your organization. > > > > > > Hi Matt, > > > > > > I implemented "Inactive the host sensors" in phosphor-hwmon use > > below > > > approaching: > > > 1. Add one option in Sensors configuration, phosphor-hwmon will > > parse > > > this option and add host sensors to _hostSensors list. > > > 2. In mainloop::read() before going to loop to read the sensors > > in the > > > reading list. Query dbus to get CurrentHostState property. > > > This property belongs to service "xyz.openbmc_project.State.Host". > > > Based on the status of this property, identify host status. > > > If the host is off, remove the host sensors from _state list and > > dbus. > > > I expected the users wouldn't see the host sensors on the BMC > > Web when > > > the host is off. > > > If the host is on, add the host sensors back to _state list and > > also dbus. > > > > > > The code is working. But I have two issues with this approaching: > > > > > > 1. Too many transactions to get dbus property CurrentHostState. > > > In my case, I have 6 services to monitor the sensors which > > concern the > > > host. > > > With the current interval 1 second of phosphor-hwmon, I have 6 > > > transactions to get CurrentHostState per seconds. > > > > The better way to implement this would be to read CurrentHostState > > once > > on startup, and then register for > > propertiesChanged signals for it and provide a callback to update an > > internal host state variable. > > > > [Thu] Yes, This is what I'm planning to do. But don't know how to add > > a propertiesChanged signals > > to the current phosphor-hwmon framework. > > > > I usually use below code to add a signal to dbus. > > boost::asio::io_service io; > > ampere::host::conn = > > std::make_shared(ampere::host::io); > > > > std::vector> matches; > > //Start tracking power state > > std::unique_ptr hostMonitor = > > ampere::host::startHostStateMonitor(); > > matches.emplace_back(std::move(hostMonitor)); > > > > /* wait for the signal */ > > ampere::host::io.run(); > > But when start io.run(), phosphor-hwmon will stop reading sensors. > > There is already an sdbusplus loop running, it's in Mainloop::run(). > > > > > > 2. When I call "ipmitool power off" the host, there is a gap > > between > > > the time I trigger GPIO to power off the chassis and the time Dbus > > > property CurrentHostState is updated. > > > In this gap, the phosphor-hwmon is still reading sensors. And this > > > causes the threshold warnings or errors. I want to avoid this. > > > > > > Do you have any suggestions to avoid these issues? > > > > > > > I can't think of anything at the moment. Maybe someone else has > > an idea > > here. > > > > > Others question: > > > I saw that phosphor-hwmon is registering an event to smbus and > > trigger > > > the event after each 1 second to read sensors. > > > Can I change the phosphor-hwmon code to integrate one dbus > > signal event? > > > Which will be triggered when there is changing in dbus property. > > > > I'm not sure I understand what you're asking for here. Right now it > > will do 1 second reads (the period is > > configurable) and send a properties changed signal for each sensor on > > D-Bus only when the value changes. > > > > [Thu] I'm asking for how to add a signal to current phosphor-hwmon > > framework as comment 1. > > > > > > > > I knew how to create a service which adds the call back function > > when > > > there is change in dbus property. > > > But don't know how to intergrace it to hwmon. > > > > > > Regards. > > > Thu Nguyen. > > > > > > > > > > > > On Fri, Oct 23, 2020 at 5:45 AM Thu Ba Nguyen > > > > > >> > > wrote: > > > > > > Just remove all of added code, rebase the phosphor-hwmon > > source to > > > commit > > > "5906173 (12 months ago) Brad Bishop: build: add support for > > > building with meson" > > > > > > Add the include: > > > #include > > > I can see the error > > > | > > > > > > /openbmc/jade_build/tmp/work/arm1176jzs-openbmc-linux-gnueabi/phosphor-hwmon/1.0+gitAUTOINC+5906173aec-r1/recipe-sysroot-native/usr/bin/arm-openbmc-linux-gnueabi/../../libexec/arm-openbmc-linux-gnueabi/gcc/arm-openbmc-linux-gnueabi/10.1.0/ld: > > > phosphor_hwmon_readd-readd.o: undefined reference to symbol > > > 'pthread_key_delete@@GLIBC_2.4' > > > | > > > > > > /openbmc/jade_build/tmp/work/arm1176jzs-openbmc-linux-gnueabi/phosphor-hwmon/1.0+gitAUTOINC+5906173aec-r1/recipe-sysroot-native/usr/bin/arm-openbmc-linux-gnueabi/../../libexec/arm-openbmc-linux-gnueabi/gcc/arm-openbmc-linux-gnueabi/10.1.0/ld: > > > > > > /openbmc/jade_build/tmp/work/arm1176jzs-openbmc-linux-gnueabi/phosphor-hwmon/1.0+gitAUTOINC+5906173aec-r1/recipe-sysroot/lib/libpthread.so.0: > > > error adding symbols: DSO missing from command line > > > | collect2: error: ld returned 1 exit status > > > | make[2]: *** [Makefile:643: phosphor-hwmon-readd] Error 1 > > > | make[2]: Leaving directory > > > > > > '/openbmc/jade_build/tmp/work/arm1176jzs-openbmc-linux-gnueabi/phosphor-hwmon/1.0+gitAUTOINC+5906173aec-r1/build' > > > | make[1]: *** [Makefile:801: all-recursive] Error 1 > > > | make[1]: Leaving directory > > > > > > '/openbmc/jade_build/tmp/work/arm1176jzs-openbmc-linux-gnueabi/phosphor-hwmon/1.0+gitAUTOINC+5906173aec-r1/build' > > > | make: *** [Makefile:524: all] Error 2 > > > | ERROR: oe_runmake failed > > > | WARNING: exit code 1 from a shell command. > > > | ERROR: Execution of > > > > > > '/openbmc/jade_build/tmp/work/arm1176jzs-openbmc-linux-gnueabi/phosphor-hwmon/1.0+gitAUTOINC+5906173aec-r1/temp/run.do_compile.2045' > > > failed with exit code 1: > > > | Makefile:800: target 'check-valgrind-recursive' given more > > than > > > once in the same rule > > > | Makefile:800: target 'check-valgrind-memcheck-recursive' > given > > > more than once in the same rule > > > | Makefile:800: target 'check-valgrind-helgrind-recursive' > given > > > more than once in the same rule > > > | Makefile:800: target 'check-valgrind-drd-recursive' given > more > > > than once in the same rule > > > | Makefile:800: target 'check-valgrind-sgcheck-recursive' given > > > more than once in the same rule > > > | make all-recursive > > > > > > I think we should add thread lib. > > > > > > Regards. > > > Thu Nguyen. > > > > > > On Thu, Oct 22, 2020 at 10:51 PM Matt Spinler > > > > > >> > > wrote: > > > > > > > > > > > > On 10/22/2020 9:49 AM, Thu Ba Nguyen wrote: > > > > I started on supporting enable/disable host sensors. > > > Everythings is > > > > fine until I... > > > > This Message Is From an External Sender > > > > This message came from outside your organization. > > > > > > > > I started on supporting enable/disable host sensors. > > > > Everythings is fine until I add code to monitor host > > status > > > as below. > > > > bool MainLoop::isHostOn(void) > > > > { > > > > char buff[128]; > > > > if (!powerMatch) > > > > { > > > > log("Power Match Not Created"); > > > > } > > > > sprintf(buff, "isHostOn powerStatusOn: > > %d\n",powerStatusOn); > > > > log(buff); > > > > return powerStatusOn; > > > > } > > > > std::shared_ptr > > > > MainLoop::startHostStateMonitor(void) { > > > > return std::make_shared( > > > > *conn, > > > > > > "type='signal',interface='org.freedesktop.DBus.Properties'," > > > > > > > > "member='PropertiesChanged',arg0='xyz.openbmc_project.State.Host'", > > > > [](sdbusplus::message::message &msg) { > > > > std::string interfaceName; > > > > boost::container::flat_map > > std::variant> > > > > propertiesChanged; > > > > try { > > > > msg.read(interfaceName, propertiesChanged); > > > > } catch (std::exception &e) { > > > > log("Unable to read host state\n"); > > > > return; > > > > } > > > > // We only want to check for CurrentHostState > > > > if (propertiesChanged.begin()->first != > > "CurrentHostState") { > > > > return; > > > > } > > > > auto findState = propertiesChanged.find(powProperty); > > > > if (findState != propertiesChanged.end()) > > > > { > > > > powerStatusOn = boost::ends_with( > > > > std::get(findState->second), "Running"); > > > > } > > > > powerMatch = true; > > > > }); > > > > } > > > > void MainLoop::read() > > > > { > > > > // TODO: Issue#3 - Need to make calls to the dbus sensor > > > cache here to > > > > // ensure the objects all exist? > > > > /* Host changed state from On to Off */ > > > > if (!isHostOn() && (lastPowerState == HOST_ON || > > > > lastPowerState == HOST_NA)) { > > > > removeHostSensors(); > > > > lastPowerState = HOST_OFF; > > > > } > > > > /* Host changed state from Off to On */ > > > > if (isHostOn() && lastPowerState == HOST_OFF) { > > > > addDroppedHostSensors(); > > > > lastPowerState = HOST_ON; > > > > } > > > > When build openBMC I got error: > > > > > > > > > > tmp/work/arm1176jzs-openbmc-linux-gnueabi/phosphor-hwmon/1.0+gitAUTOINC+5906173aec-r1/recipe-sysroot/lib/libpthread.so.0: > > > > > > > error adding symbols: DSO missing from command line > > > > | collect2: error: ld returned 1 exit status > > > > | make[2]: *** [Makefile:643: phosphor-hwmon-readd] > > Error 1 > > > > > > > > It seems I need adding the threads lib to defend lib. > > > > Any suggestion to add threads lib to build configuration? > > > > > > > > > > That must be because you're using that single boost > > function? > > > While you > > > could add the dependency, > > > the ideal thing to do since this repo already uses > > > phosphor-dbus-interfaces is to use the function: > > > > > > /** @brief Convert a string to an appropriate > > enum value. > > > * @param[in] s - The string to convert in the > > form of > > > * "xyz.openbmc_project.State.Host." > > > * @return - The enum value. > > > */ > > > static HostState convertHostStateFromString(const > > > std::string& s); > > > > > > to convert it to the actual HostState enum to check > against: > > > > > > enum class HostState > > > { > > > Off, > > > Running, > > > Quiesced, > > > DiagnosticMode, > > > }; > > > > > > This is all in xyz/openbmc_project/State/Host/server.hpp > > > provided by > > > phosphor-dbus-interfaces. > > > > > > > Thanks. > > > > Thu. > > > > > > > > On Wed, Oct 21, 2020 at 11:54 PM Thu Ba Nguyen > > > > > > > > > > > > > > > >>> wrote: > > > > > > > > Hi Vijay, > > > > > > > > I took a look on entity-manager and openbmc source. > > > > Don't have many companies using entity-manager > > model to > > > support > > > > sensors. > > > > > > > > Regards > > > > Thu Nguyen. > > > > > > > > > > > > On Wed, Oct 21, 2020 at 7:15 AM Vijay Khemka > > > > > > > > > > > > >>> > > > wrote: > > > > > > > > *From: *openbmc > > > > > > > > > > > > > > > > > > > > > >>> on behalf of Thu Ba Nguyen > > > > > > > > > > > > > > > >>> > > > > *Date: *Monday, October 19, 2020 at 11:23 AM > > > > *To: *Ed Tanous > > > > > > > > > > >>> > > > > *Cc: *OpenBMC Maillist > > > > > > > > > > > > > > > > >>> > > > > *Subject: *Re: Enable/Disable some sensors > > when Host > > > On/Off > > > > > > > > Hi Ed Tanous, > > > > > > > > > Thanks for your info, > > > > > > > > > But in your platform we are using > phosphor-hwmon > > > to manage > > > > sensors. > > > > > > > > > We don't use entity-manager. > > > > > > > > > As I knew we can't use both entity-manager and > > > > phosphor-hwmon for one project. > > > > > > > > You can use both but for different sensors. > > You can > > > decide > > > > what sensors to configure > > > > > > > > via EM/dbus-sensors and which one for > > phosphor-hwmon. > > > > > > > > Regards > > > > > > > > Thu Nguyen. > > > > > > > > > > >