utk-robotics-2017 / rip

RIP v1 is now deprecated, please move to RIP v2
https://gitlab.com/utk-robotics
6 stars 2 forks source link

Periphery/dev #36

Closed ilumsden closed 6 years ago

ilumsden commented 6 years ago

Adds the C++ wrapping for periphery (peripherycpp).

robobenklein commented 6 years ago

Although there are zero tests currently, https://github.com/utk-robotics-2017/rip/pull/36/commits/b06ac6228572ba8e27b4a7337ea4c642c7ebedf3 sets the blank to be tested. Once Doxygen comments are good, :+1: from me.

ilumsden commented 6 years ago

Just a note: Doxygen comments have been added to all the hpp files since the pull request was opened.

xxAtrain223 commented 6 years ago

It would be nice if there was a periphery abstract class to provide a unified interface.

For example:

class Device
{
public:
    std::vector<uint8_t> readBytes() = 0;
    std::string readString() = 0;
    void write(std::vector<uint8_t> data) = 0;
    void write(std::string str) = 0;
    void close() = 0;
    ...
};

class Serial : public Device
{
public:
    void write(std::string str)
    {
        this->write(std::vector<uint8_t>(str.begin(), str.end()));
    }
    ...
};

class Mmio ; public Device
{
public:
    void write(std::string str)
    {
        this->write(0, std::vector<uint8_t>(str.begin(), str.end()));
    }
    ...
};

int main(int argc, char** argv)
{
    Device serial = Serial(/* Connection settings */);
    Device mmio = Mmio(/* Connection settings*/);

    serial.write("Hello");
    mmio.write("Hello");
}
jptech commented 6 years ago

For bookkeeping... resolves #28

codecov[bot] commented 6 years ago

Codecov Report

Merging #36 into dev will increase coverage by 8.32%. The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff            @@
##              dev    #36      +/-   ##
========================================
+ Coverage   91.67%   100%   +8.32%     
========================================
  Files          69      1      -68     
  Lines        4361      3    -4358     
========================================
- Hits         3998      3    -3995     
+ Misses        363      0     -363
Impacted Files Coverage Δ
core/utilities/peripherycpp/test/test_main.cpp 100% <100%> (ø)
core/motor_controllers/src/config.cpp
...ies/cmd_messenger/include/cmd_messenger/device.hpp
arduino_gen/test/test_constructor.cpp
arduino_gen/include/arduino_gen/setup.hpp
arduino_gen/src/xml_utils.cpp
core/motor_controllers/test/test_roboclaw.cpp
arduino_gen/src/loop.cpp
...lude/motor_controllers/roboclaw/pid_parameters.hpp
arduino_gen/include/arduino_gen/xml_element.hpp
... and 61 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 85c7221...a899cc8. Read the comment docs.

jptech commented 6 years ago

Primary things to check/fix before merging:

argvrutter commented 6 years ago

Let's do some hardware testing today, make sure at the very least the most commonly used functions work for gpio, serial, & i2c.

argvrutter commented 6 years ago
    void I2c::transfer(std::vector< std::vector<uint8_t> > msg_data, std::vector<int> flags, size_t count)

Any reason why it's a vector of vectors instead of just one vector?

ilumsden commented 6 years ago

@solsane, there are two reasons why it's a vector of vectors. First, it allows the function to send multiple messages with one function call. Second, it allows each uint8_t vector (the message) to be accessed with the same index as the corresponding flag.