Add Sensor API

[zuckschwerdt]

This is good to merge, @vsonnier @guruofquality can you take a quick glance? Things can be improved and generalized later, particularly I'm not happy with the locale hoops to jump for strtod, but it should work without problems already. Example output is in #22 and I'll PR those enhancements to SoapySDRUtil shortly.

[vsonnier]

Hello @zuckschwerdt @guruofquality, I tried the feat-sensors branch on Windows x64 but unfortunatly I doesn't have any sensor reading on the command SoapySDRUtil.exe --probe="driver=plutosdr":

----------------------------------------------------
-- Peripheral summary
----------------------------------------------------
  Channels: 1 Rx, 1 Tx
  Timestamps: NO
  Sensors: xadc_temp0, xadc_voltage0, xadc_voltage1, xadc_voltage2, xadc_voltage3, xadc_voltage4, xadc_voltage5, xadc_voltage6, xadc_voltage7, xadc_voltage8, adm1177_current0, adm1177_voltage0, ad9361-phy_temp0, ad9361-phy_voltage2

----------------------------------------------------
-- RX Channel 0
----------------------------------------------------

Putting traces, it seems neither SoapySDR::ArgInfo SoapyPlutoSDR::getSensorInfo(const std::string &key) const nor std::string SoapyPlutoSDR::readSensor(const std::string &key) const get called.

On a side note, indeed strtod and program-wide setlocale make my heart an eyes bleed :) please replace it by someting like that:

double SoapyPlutoSDR::get_sensor_value(struct iio_channel *chn) const
{
    double val = 0.0;
    char buf[32];

    std::stringstream val_as_string;

    if (iio_channel_find_attr(chn, "input")) {
        if (iio_channel_attr_read(chn, "input", buf, sizeof(buf)) > 0) {
            val_as_string << buf;
            val_as_string >> val;
        }
    }
    else {
        if (iio_channel_attr_read(chn, "raw", buf, sizeof(buf)) > 0) {
            val_as_string << buf;
            val_as_string >> val;
        }
    }

    if (iio_channel_find_attr(chn, "offset")) {
        if (iio_channel_attr_read(chn, "offset", buf, sizeof(buf)) > 0) {
            std::stringstream offset_as_string;
            double offset = 0.0;
            offset_as_string << buf;
            offset_as_string >> offset;
            val += offset;
        }
    }

    if (iio_channel_find_attr(chn, "scale")) {
        if (iio_channel_attr_read(chn, "scale", buf, sizeof(buf)) > 0) {
            std::stringstream scale_as_string;
            double scale = 1.0;
            scale_as_string << buf;
            scale_as_string >> scale;       
            val *= scale;
        }
    }

    return val / 1000.0;
}
(with #include <iostream>)

Also:

std::string SoapyPlutoSDR::id_to_unit(const std::string& id) const
{
    static std::map<std::string, std::string> id_to_unit_table = {
        { "current",    "A" },
        { "power",  "W" },
        { "temp",   "C" },
        { "voltage",    "V" }
    };

    auto it_match = id_to_unit_table.find(id);

    if (it_match != id_to_unit_table.end()) {

        return it_match->second;
    }

    return std::string();
}
(with #include <map>)

C++11 constructor initializers FTW.

Not tested, since the related functions are not called but that's the idea.

[zuckschwerdt]

Thanks! Will do. SoapySDRUtil needs a patch to show the values. I've pushed a preliminary commit to SoapySDR/feat-sensors

From what I've read using streams instead of strtod is noticably slower and I'm not sure what the locale on the stream is set to?

[vsonnier]

SoapySDRUtil needs a patch to show the values. I've pushed a preliminary commit to SoapySDR/feat-sensors.

I've tested this branch. However, it doesn't compile because sleep(1) is not standard C++. I replaced it with

#include <chrono>
#include <thread>
...
std::this_thread::sleep_for(std::chrono::seconds(1));

At that point, I was able to run the intended command, which leads me to really understand id_to_unit, so here the definitive version:

std::string SoapyPlutoSDR::id_to_unit(const std::string& id) const
{
    static std::map<std::string, std::string> id_to_unit_table = {
        { "current",    "A" },
        { "power",  "W" },
        { "temp",   "C" },
        { "voltage",    "V" }
    };

    for (auto it_match : id_to_unit_table) {

        //if the id starts with a known prefix, retreive its unit.
        if (id.substr(0, it_match.first.size()) == it_match.first) {
            return it_match.second;
        }
    }
    return std::string();
}

( [/comment nazi]Of the importance of comments in the code...[comment nazi/])

Finally I got:

----------------------------------------------------
-- Peripheral summary
----------------------------------------------------
  Channels: 1 Rx, 1 Tx
  Timestamps: NO
  Sensors: xadc_temp0, xadc_voltage0, xadc_voltage1, xadc_voltage2, xadc_voltage3, xadc_voltage4, xadc_voltage5, xadc_voltage6, xadc_voltage7, xadc_voltage8, adm1177_current0, adm1177_voltage0, ad9361-phy_temp0, ad9361-phy_voltage2
     * xadc_temp0: 53.030573 C
     * xadc_voltage0 (vccint): 1.012207 V
     * xadc_voltage1 (vccaux): 1.796631 V
     * xadc_voltage2 (vccbram): 1.011475 V
     * xadc_voltage3 (vccpint): 1.003418 V
     * xadc_voltage4 (vccpaux): 1.795166 V
     * xadc_voltage5 (vccoddr): 1.343262 V
     * xadc_voltage6 (vrefp): 1.249512 V
     * xadc_voltage7 (vrefn): 0.002197 V
     * xadc_voltage8: 0.897705 V
     * adm1177_current0: 0.404499 A
     * adm1177_voltage0: 4.914893 V
     * ad9361-phy_temp0: 35.965000 C
     * ad9361-phy_voltage2: 0.191392 V

----------------------------------------------------
-- RX Channel 0
----------------------------------------------------

From what I've read using streams instead of strtod is noticably slower and I'm not sure what the locale on the stream is set to?

Well perfomance is not an issue here, clarity is. And for about the locale, don't touch it especially for simple things as converting strings to numbers.

In any case both PR have to be merged "together" else this module-only modification is of small use by itself.

Edit: SoapySDRUtil Looks Good To Me.

[guruofquality]

@zuckschwerdt this PR looks good, please merge when you feel its ready.

Also, is feat-sensors ready, can you make another PR for that one?

[zuckschwerdt]

Well perfomance is not an issue here, clarity is. And for about the locale, don't touch it especially for simple things as converting strings to numbers.

I like neither strtod nor going through another wrapper (stringstream) with ~4x speed penalty. But I tend to prefer simple and obvious code too :)

I'm not confident about the locale though. libiio will always return numbers according to "C" locale. Is the stream guaranteed to read in "C" locale even if there is a different (system) locale set? (floating point numbers aren't that simple when you can't be sure what the decimal point character is.) I tested this superficially and while strtod picks up the set locale, the stringstream didn't -- I failed to dig up information what locale the basic_stringstream constructor (or rather basic_ios::init) uses?

[vsonnier]

Hello Christian,

The main issue of the old C setlocale is that it is program-wide, affects all functions calls without any kind of locking at that, so should be forbidden except to set it once at application startup.

In C++ you can on the other hand have local locales (pun intended) proper to the stream you are using. If you want to force the "C" locale equivalent apparently you can set it that way:

val_as_string.imbue(std::locale::classic());

[zuckschwerdt]

Agreed. The question now though, do we need to call imbue on a stringstream, what is the default?

[zuckschwerdt]

Ah, ok. Yes we need that call to be sure. std::basic_ios::init() uses std::locale() which uses the global C++ locale, and that could be anything.

[vsonnier]

That is looking fine to me.