2

As an embedded developer, I often write drivers for hardware (though this question really applies to any shared resource). The "standard" interface I have come up with looks like the following. As an example, I will showing an interface for a "Voltage Probe."

class IVoltageProbe
{
public:
    virtual void enable() = 0;
    virtual void disable() = 0;
    virtual units::Volts read() = 0;
};

The standard "enable" function allows the user to place the driver into a state such that it is ready to to perform its intended function. The "disable" function places the driver (and associated hardware) into its lowest-power, uninitialized, etc. state.

This has been working well across my code until now.

I needed to implement two different Voltage Probes to be used from independent threads. In this case, they are implemented as using ADC channels. However, even though each probe is a separate channel, they must both be part of the same ADC hardware block. In addition, there is no way to independently "read" from on channel without affecting the configuration of the other.

Thus, for example, if I provide a simple implementation of "enable" and "disable" that actually enables and disables the underlying ADC hardware, this will affect the state of the other voltage probe. The single ADC is a shared hardware resource.

One possible solution I thought of to this problem involves a "proxy" driver which

  1. Keeps track of all the upper-level voltage probes and their enabled/disabled states
  2. Enables the underlying hardware if any of the probes is enabled
  3. Disables the underlying hardware if all the probes are disabled
  4. Provides some form of mutual exclusion (e.g., just use a mutex)

This solution ends up making what once was simple code into something much more complex.

What are some other solutions to this type of problem? Do I need to consider redesigning my abstract interface?

1
  • 5
    You can't model away complexity. You have what is actually a shared resource (the ADC), you need something to handle that - personally, your proxy driver solution sounds good to me. It's something you need to write once and can then forget about. Oct 13, 2023 at 14:30

3 Answers 3

2

Your proposed solution is sound.

It is probably necessary to design the drivers like you scetched it for dealing with this kind of hardware. This keeps the complexity internal to the drivers, so users of the interface ideally won't notice that the two different probes are not as independent as the interface pretends.

However, parts of this abstraction may leak to the caller. enable may not really enable the channel when the common "ADC hardware block" (whatever that is) was already enabled by another channel, and it may not really disable a channel as long as the common hardware is still in use by another one. That might lead to different behaviour in case of transmission errors, or - worse - could lead to unwanted side effects, where operations on one channel might influence the other.

If that's the case, you cannot really prevent it, since the potential for side-effects is build into the hardware. What you can do, however, is making this behaviour more transparent to the caller. This could be achieved by

  • renaming the methods enable and disable to something which expresses more precisely what really happens under the hood (maybe something like tryEnable/ tryDisable, or registerChannel and unregisterChannel)

  • add a method which allows to check the "real" current enabling state by the caller

  • ade more methods for checking the shared state of the underlying hardware, in case it is necessary.

Note I am not saying you should redesign you interface "just in case" - if it works well as it designed for now, leave it as it is. Only consider a redesign when you cannot prevent the abstraction of the shared state leaking to the caller.

2

For future reference, I'd like to share a demo of the solution I implemented in my code (this is not my actual code, but an example you can compile on a standard PC).

#include <list>
#include <iostream>
#include <mutex>
#include <cassert>
#include <algorithm>

//==============================================================================

namespace units
{
using Volts = unsigned int;
}

using Mutex = std::mutex;
using LockGuard = std::lock_guard<Mutex>;

//==============================================================================

/**
* @brief Driver for ADC hardware
*/
class ADC
{
public:
    enum class Channel
    {
        CHANNEL_1,
        CHANNEL_2,
        CHANNEL_3
    };
    
public:
    ADC():
        _enabled(false)
    {
        std::cout << __PRETTY_FUNCTION__ << std::endl;
    }
    ~ADC()
    {
        std::cout << __PRETTY_FUNCTION__ << std::endl;
        disable();
    }

public:
    void enable()
    {
        if (isEnabled()) return;
            
        std::cout << "\033[1;32m" << __PRETTY_FUNCTION__ << "\033[1;39m" << std::endl;
        _enabled = true;
    }
    
    void disable()
    {
        if (!isEnabled()) return;
            
        std::cout << "\033[1;31m" << __PRETTY_FUNCTION__ << "\033[1;39m" << std::endl;
        _enabled = false;
    }
    
    bool isEnabled()
    {
        return _enabled;
    }
    
    units::Volts read(Channel c)
    {
        std::cout << "\033[1;34m" << __PRETTY_FUNCTION__ << "\033[1;39m" << std::endl;
            
        assert(isEnabled());
            
        switch(c)
        {
        case Channel::CHANNEL_1:
            return 10;
        case Channel::CHANNEL_2:
            return 20;
        case Channel::CHANNEL_3:
            return 30;
        default:
            return 0;
        }
    }
        
private:
    bool _enabled;
};

class ManagedChannelProbe;

/**
* @brief Wrapper which allows ManagedChannelProbe instances to access an ADC
*        driver in a thread-safe manner
*/
class ADCManager
{
public:
    ADCManager(ADC& adc):
        _adc(adc)
    {
        std::cout << __PRETTY_FUNCTION__ << std::endl;
    }
    
    ~ADCManager()
    {
        std::cout << __PRETTY_FUNCTION__ << std::endl;
    }
    
public:
    void enable(ManagedChannelProbe* p)
    {
        LockGuard lock(_mutex);
        
        auto it = std::find(_probes.begin(), _probes.end(), p);
        if (it == _probes.end())
        {
            _probes.push_back(p);
            if (!_adc.isEnabled())
            {
                _adc.enable();
            }
        }
    }
    
    void disable(ManagedChannelProbe* p)
    {
        LockGuard lock(_mutex);
        
        _probes.remove(p);
        //Disable the underlying ADC driver if no probes are enabled
        if (_probes.size() == 0)
        {
            if (_adc.isEnabled())
            {
                _adc.disable();
            }
        }
    }
    
    units::Volts read(ManagedChannelProbe* p, ADC::Channel channel)
    {
        LockGuard lock(_mutex);
        
        auto it = std::find(_probes.begin(), _probes.end(), p);
        assert(it != _probes.end());
        
        return _adc.read(channel);
    }
    
private:
    mutable Mutex _mutex;
    ADC& _adc;
    /*
    * Since this list only stores pointers, it could be implemented more
    * efficiently as an array of pointers with unused indeces set to nullptr
    */
    std::list<ManagedChannelProbe*> _probes;
};

/**
* @brief Interface for a "voltage probe"
*/
class IVoltageProbe
{
public:
    virtual void enable() = 0;
    virtual void disable() = 0;
    virtual units::Volts read() = 0;
};

/**
* @brief A voltage probe which is via an ADCManager
*/
class ManagedChannelProbe : public IVoltageProbe
{
public:
    ManagedChannelProbe(ADCManager& adc, ADC::Channel channel):
        _adc(adc)
        ,_channel(channel)
    {
        std::cout << __PRETTY_FUNCTION__ << std::endl;
    }

    ~ManagedChannelProbe()
    {
        std::cout << __PRETTY_FUNCTION__ << std::endl;
            
        //This ensures that this probe is "deregistered" from the ADCManager
        disable();
    }

public:
    void enable() override
    {
        _adc.enable(this);
    }
    
    void disable() override
    {
        _adc.disable(this);
    }
    
    units::Volts read() override
    {
        return _adc.read(this, _channel);
    }
    
private:
    ADCManager& _adc;
    ADC::Channel _channel;
};

int main()
{
    ADC adc;
    ADCManager adcManager(adc);
    
    ManagedChannelProbe probe1(adcManager, ADC::Channel::CHANNEL_1);
    ManagedChannelProbe probe2(adcManager, ADC::Channel::CHANNEL_2);
    
    //Example of interleaved acesses to the ADC hardware
    probe1.enable();
    probe2.enable();
    const auto v1 = probe1.read();
    probe1.disable();
    const auto v2 = probe2.read();
    probe2.disable();
}

The ADC class is the real/original hardware driver. The ADCManager class is a proxy which "manages" the multi-user, cross-thread access to the driver. I just use a standard mutex to prevent multiple concurrent calls to the same driver function. This works in my case, but may not depending on the implementation of the driver (e.g., a "unique handle" which can be "obtained" by the managed locks the driver until it is "released"). The IVoltageProbe class is the original interface used by the rest of the code (I wanted to "inject" the proxy without changing the rest of my application code). Finally, the ManagedChannelProbe class implements the IVoltageProbe interface and uses the the ADCManager proxy to access the real hardware.

In the main function, I for the enable and disable calls to be interleaved. Without the proxy driver, the ADC hardware would be disabled by the time probe2.read is called at which point an assert would be triggered.

You should see the following output if you compile and run this code:

ADC::ADC()
ADCManager::ADCManager(ADC&)
ManagedChannelProbe::ManagedChannelProbe(ADCManager&, ADC::Channel)
ManagedChannelProbe::ManagedChannelProbe(ADCManager&, ADC::Channel)
void ADC::enable()
units::Volts ADC::read(ADC::Channel)
units::Volts ADC::read(ADC::Channel)
void ADC::disable()
ManagedChannelProbe::~ManagedChannelProbe()
ManagedChannelProbe::~ManagedChannelProbe()
ADCManager::~ADCManager()
ADC::~ADC()
2

Operating systems which allow shared access to resources such as disks etc. often use a reference count to note how many processes opened a device. The first one to open() it would actually enable the hardware and set it up for operation, while the last one to close() the device would cause it to be shut down. By providing open()and close(), your driver would avoid using the concept of enable()/disable() which imply changes to the device state that may affect the operation of other processes which are using the device.

Of course, in an embedded system you might not be able to rely on proper open/close semantics on the process level. Using C++ or other languages you can use constructor/destructor semantics to maintain the reference count, which should work well unless you have really fatal exceptions.

2
  • If I understand correctly, the idea is, since enable/disable semantically imply that the state of the device/driver changes (i.e., is enabled/is disabled) BUT the nature of the shared resource means that this might not actually be true (e.g., enable will do nothing if the device is already enabled), the work should be move to a "3rd party" (i.e., OS). Essentially, the 3rd party returns a "handle" to a driver that is guaranteed to be "enabled." Oct 16, 2023 at 19:03
  • 1
    @PatrickWright essentially, yes. It does not need to be an OS, but the driver can manage it. To me, it's mostly a matter of naming - open/close implies does not imply state change as much as enable/disable does. Oct 17, 2023 at 7:54

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.