From be077713b3e8c6ba8f06523f7c846a5be0a96584 Mon Sep 17 00:00:00 2001 From: George Beckstein Date: Tue, 23 Feb 2021 16:19:24 -0500 Subject: [PATCH 1/5] Implement polymorphism for CAN --- drivers/include/drivers/CAN.h | 115 ++---------- .../include/drivers/interfaces/InterfaceCAN.h | 171 ++++++++++++++++++ drivers/source/CAN.cpp | 2 +- 3 files changed, 184 insertions(+), 104 deletions(-) create mode 100644 drivers/include/drivers/interfaces/InterfaceCAN.h diff --git a/drivers/include/drivers/CAN.h b/drivers/include/drivers/CAN.h index 66d6ad422d6..e1e40b72a7f 100644 --- a/drivers/include/drivers/CAN.h +++ b/drivers/include/drivers/CAN.h @@ -21,91 +21,13 @@ #if DEVICE_CAN || defined(DOXYGEN_ONLY) +#include "interfaces/InterfaceCAN.h" #include "hal/can_api.h" #include "platform/Callback.h" #include "platform/PlatformMutex.h" #include "platform/NonCopyable.h" namespace mbed { -/** \defgroup drivers-public-api-can CAN - * \ingroup drivers-public-api - */ - -/** - * \defgroup drivers_CANMessage CANMessage class - * \ingroup drivers-public-api-can - * @{ - */ - -/** CANMessage class - * - * @note Synchronization level: Thread safe - */ -class CANMessage : public CAN_Message { - -public: - /** Creates empty CAN message. - */ - CANMessage() : CAN_Message() - { - len = 8U; - type = CANData; - format = CANStandard; - id = 0U; - memset(data, 0, 8); - } - - /** Creates CAN message with specific content. - * - * @param _id Message ID - * @param _data Mesaage Data - * @param _len Message Data length - * @param _type Type of Data: Use enum CANType for valid parameter values - * @param _format Data Format: Use enum CANFormat for valid parameter values - */ - CANMessage(unsigned int _id, const unsigned char *_data, unsigned char _len = 8, CANType _type = CANData, CANFormat _format = CANStandard) - { - len = (_len > 8) ? 8 : _len; - type = _type; - format = _format; - id = _id; - memcpy(data, _data, len); - } - - - /** Creates CAN message with specific content. - * - * @param _id Message ID - * @param _data Mesaage Data - * @param _len Message Data length - * @param _type Type of Data: Use enum CANType for valid parameter values - * @param _format Data Format: Use enum CANFormat for valid parameter values - */ - CANMessage(unsigned int _id, const char *_data, unsigned char _len = 8, CANType _type = CANData, CANFormat _format = CANStandard) - { - len = (_len > 8) ? 8 : _len; - type = _type; - format = _format; - id = _id; - memcpy(data, _data, len); - } - - /** Creates CAN remote message. - * - * @param _id Message ID - * @param _format Data Format: Use enum CANType for valid parameter values - */ - CANMessage(unsigned int _id, CANFormat _format = CANStandard) - { - len = 0; - type = CANRemote; - format = _format; - id = _id; - memset(data, 0, 8); - } -}; - -/** @}*/ /** * \defgroup drivers_CAN CAN class @@ -115,7 +37,13 @@ class CANMessage : public CAN_Message { /** A can bus client, used for communicating with can devices */ -class CAN : private NonCopyable { +class CAN +#ifdef FEATURE_EXPERIMENTAL_API + final : public interface::CAN, private NonCopyable +#else + : private NonCopyable +#endif +{ public: /** Creates a CAN interface connected to specific pins. @@ -233,14 +161,7 @@ class CAN : private NonCopyable { */ void monitor(bool silent); - enum Mode { - Reset = 0, - Normal, - Silent, - LocalTest, - GlobalTest, - SilentTest - }; + using Mode = ::mbed::interface::can::Mode; /** Change CAN operation to the specified mode * @@ -277,19 +198,7 @@ class CAN : private NonCopyable { */ unsigned char tderror(); - enum IrqType { - RxIrq = 0, - TxIrq, - EwIrq, - DoIrq, - WuIrq, - EpIrq, - AlIrq, - BeIrq, - IdIrq, - - IrqCnt - }; + using IrqType = ::mbed::interface::can::IrqType; /** Attach a function to call whenever a CAN frame received interrupt is * generated. @@ -299,7 +208,7 @@ class CAN : private NonCopyable { * @param func A pointer to a void function, or 0 to set as none * @param type Which CAN interrupt to attach the member function to (CAN::RxIrq for message received, CAN::TxIrq for transmitted or aborted, CAN::EwIrq for error warning, CAN::DoIrq for data overrun, CAN::WuIrq for wake-up, CAN::EpIrq for error passive, CAN::AlIrq for arbitration lost, CAN::BeIrq for bus error) */ - void attach(Callback func, IrqType type = RxIrq); + void attach(Callback func, IrqType type = IrqType::RxIrq); static void _irq_handler(uint32_t id, CanIrqType type); @@ -309,7 +218,7 @@ class CAN : private NonCopyable { virtual void unlock(); can_t _can; - Callback _irq[IrqCnt]; + Callback _irq[IrqType::IrqCnt]; PlatformMutex _mutex; #endif }; diff --git a/drivers/include/drivers/interfaces/InterfaceCAN.h b/drivers/include/drivers/interfaces/InterfaceCAN.h new file mode 100644 index 00000000000..543270b9ca8 --- /dev/null +++ b/drivers/include/drivers/interfaces/InterfaceCAN.h @@ -0,0 +1,171 @@ +/* + * Mbed-OS Microcontroller Library + * Copyright (c) 2021 Embedded Planet + * Copyright (c) 2021 ARM Limited + * SPDX-License-Identifier: Apache-2.0 + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License + */ + +#ifndef MBED_INTERFACE_CAN_H_ +#define MBED_INTERFACE_CAN_H_ + +#include "hal/can_helper.h" + +#include + +#include "platform/Callback.h" + +namespace mbed { + +// Forward declare CAN +class CAN; + +/** \defgroup drivers-public-api-can CAN + * \ingroup drivers-public-api + */ + +/** + * \defgroup drivers_CANMessage CANMessage class + * \ingroup drivers-public-api-can + * @{ + */ + +/** CANMessage class + * + * @note Synchronization level: Thread safe + */ +class CANMessage : public CAN_Message { + +public: + /** Creates empty CAN message. + */ + CANMessage() : CAN_Message() + { + len = 8U; + type = CANData; + format = CANStandard; + id = 0U; + memset(data, 0, 8); + } + + /** Creates CAN message with specific content. + * + * @param _id Message ID + * @param _data Mesaage Data + * @param _len Message Data length + * @param _type Type of Data: Use enum CANType for valid parameter values + * @param _format Data Format: Use enum CANFormat for valid parameter values + */ + CANMessage(unsigned int _id, const unsigned char *_data, unsigned char _len = 8, CANType _type = CANData, CANFormat _format = CANStandard) + { + len = (_len > 8) ? 8 : _len; + type = _type; + format = _format; + id = _id; + memcpy(data, _data, len); + } + + + /** Creates CAN message with specific content. + * + * @param _id Message ID + * @param _data Mesaage Data + * @param _len Message Data length + * @param _type Type of Data: Use enum CANType for valid parameter values + * @param _format Data Format: Use enum CANFormat for valid parameter values + */ + CANMessage(unsigned int _id, const char *_data, unsigned char _len = 8, CANType _type = CANData, CANFormat _format = CANStandard) + { + len = (_len > 8) ? 8 : _len; + type = _type; + format = _format; + id = _id; + memcpy(data, _data, len); + } + + /** Creates CAN remote message. + * + * @param _id Message ID + * @param _format Data Format: Use enum CANType for valid parameter values + */ + CANMessage(unsigned int _id, CANFormat _format = CANStandard) + { + len = 0; + type = CANRemote; + format = _format; + id = _id; + memset(data, 0, 8); + } +}; + +/** @}*/ + +namespace interface { + +namespace can { + +enum Mode { + Reset = 0, + Normal, + Silent, + LocalTest, + GlobalTest, + SilentTest +}; + +enum IrqType { + RxIrq = 0, + TxIrq, + EwIrq, + DoIrq, + WuIrq, + EpIrq, + AlIrq, + BeIrq, + IdIrq, + + IrqCnt +}; + +} // namespace can + +#ifdef FEATURE_EXPERIMENTAL_API + +// Pure virtual interface for CAN +struct CAN { + + using Mode = ::mbed::interface::can::Mode; + using IrqType = ::mbed::interface::can::IrqType; + + virtual ~CAN() = default; + virtual int frequency(int hz) = 0; + virtual int write(CANMessage msg) = 0; + virtual int read(CANMessage &msg, int handle = 0) = 0; + virtual void reset() = 0; + virtual void monitor(bool silent) = 0; + virtual int mode(Mode mode) = 0; + virtual int filter(unsigned int id, unsigned int mask, CANFormat format = CANAny, int handle = 0) = 0; + virtual unsigned char rderror() = 0; + virtual unsigned char tderror() = 0; + virtual void attach(Callback func, IrqType type = IrqType::RxIrq) = 0; +}; + +#else +using CAN = ::mbed::CAN; +#endif + +} // namespace interface +} // namespace mbed + +#endif /* MBED_INTERFACE_CAN_H_ */ diff --git a/drivers/source/CAN.cpp b/drivers/source/CAN.cpp index 6688829c8eb..2767c0743c7 100644 --- a/drivers/source/CAN.cpp +++ b/drivers/source/CAN.cpp @@ -55,7 +55,7 @@ CAN::~CAN() // No lock needed in destructor // Detaching interrupts releases the sleep lock if it was locked - for (int irq = 0; irq < IrqCnt; irq++) { + for (int irq = 0; irq < IrqType::IrqCnt; irq++) { attach(nullptr, (IrqType)irq); } can_irq_free(&_can); From d6104c8194e122b6a79dceebc98a1f23e919520c Mon Sep 17 00:00:00 2001 From: George Beckstein Date: Tue, 4 May 2021 01:25:16 -0400 Subject: [PATCH 2/5] Enable inheritance of CAN enum types This commit changes the `interface::can` namespace to a `struct`. This allows the enum types to be inherited and prevents breaking old code relying on referencing eg: `CAN::RxIrq`. When enabled, the polymorphic CAN interface class inherits from this `interface::can` struct. If not enabled, the `mbed::CAN` class inherits from `interface::can` directly. Co-authored-by: Vincent Coubard --- drivers/include/drivers/CAN.h | 6 +----- .../include/drivers/interfaces/InterfaceCAN.h | 17 +++++++++++------ 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/drivers/include/drivers/CAN.h b/drivers/include/drivers/CAN.h index e1e40b72a7f..c5621f73133 100644 --- a/drivers/include/drivers/CAN.h +++ b/drivers/include/drivers/CAN.h @@ -41,7 +41,7 @@ class CAN #ifdef FEATURE_EXPERIMENTAL_API final : public interface::CAN, private NonCopyable #else - : private NonCopyable + : private NonCopyable, public interface::can #endif { @@ -161,8 +161,6 @@ class CAN */ void monitor(bool silent); - using Mode = ::mbed::interface::can::Mode; - /** Change CAN operation to the specified mode * * @param mode The new operation mode (CAN::Normal, CAN::Silent, CAN::LocalTest, CAN::GlobalTest, CAN::SilentTest) @@ -198,8 +196,6 @@ class CAN */ unsigned char tderror(); - using IrqType = ::mbed::interface::can::IrqType; - /** Attach a function to call whenever a CAN frame received interrupt is * generated. * diff --git a/drivers/include/drivers/interfaces/InterfaceCAN.h b/drivers/include/drivers/interfaces/InterfaceCAN.h index 543270b9ca8..a73eceae427 100644 --- a/drivers/include/drivers/interfaces/InterfaceCAN.h +++ b/drivers/include/drivers/interfaces/InterfaceCAN.h @@ -113,7 +113,8 @@ class CANMessage : public CAN_Message { namespace interface { -namespace can { +/* Having this as a struct allows interface::CAN and/or mbed::CAN to inherit the enums */ +struct can { enum Mode { Reset = 0, @@ -138,15 +139,19 @@ enum IrqType { IrqCnt }; -} // namespace can +// Prevent slicing and user creation of base class. +protected: + can() = default; + ~can() = default; + can(const can&) = default; + can& operator=(const can&) = default; + +}; #ifdef FEATURE_EXPERIMENTAL_API // Pure virtual interface for CAN -struct CAN { - - using Mode = ::mbed::interface::can::Mode; - using IrqType = ::mbed::interface::can::IrqType; +struct CAN : public interface::can { virtual ~CAN() = default; virtual int frequency(int hz) = 0; From 49e58ddab64459773683b51189eeeb19fe72ffd8 Mon Sep 17 00:00:00 2001 From: George Beckstein Date: Tue, 4 May 2021 02:23:51 -0400 Subject: [PATCH 3/5] Make interface::CAN buildable on targets without DEVICE_CAN This commit adds provisions to enable using interface::CAN on targets that don't have DEVICE_CAN set to 1 (ie: they don't normally have a CAN peripheral). --- drivers/include/drivers/interfaces/InterfaceCAN.h | 7 +++++++ hal/include/hal/can_helper.h | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/include/drivers/interfaces/InterfaceCAN.h b/drivers/include/drivers/interfaces/InterfaceCAN.h index a73eceae427..6429b00ede2 100644 --- a/drivers/include/drivers/interfaces/InterfaceCAN.h +++ b/drivers/include/drivers/interfaces/InterfaceCAN.h @@ -28,8 +28,10 @@ namespace mbed { +#ifndef FEATURE_EXPERIMENTAL_API // Forward declare CAN class CAN; +#endif /** \defgroup drivers-public-api-can CAN * \ingroup drivers-public-api @@ -171,6 +173,11 @@ using CAN = ::mbed::CAN; #endif } // namespace interface + +#if defined(FEATURE_EXPERIMENTAL_API) && !DEVICE_CAN +using CAN = interface::CAN; +#endif + } // namespace mbed #endif /* MBED_INTERFACE_CAN_H_ */ diff --git a/hal/include/hal/can_helper.h b/hal/include/hal/can_helper.h index 3f56f16c6fa..c9a66981daa 100644 --- a/hal/include/hal/can_helper.h +++ b/hal/include/hal/can_helper.h @@ -20,7 +20,7 @@ #ifndef MBED_CAN_HELPER_H #define MBED_CAN_HELPER_H -#if DEVICE_CAN +#if DEVICE_CAN || FEATURE_EXPERIMENTAL_API #ifdef __cplusplus extern "C" { From d79446c0daf16c1daf7ea5ade8e2ca31f9f15b61 Mon Sep 17 00:00:00 2001 From: George Beckstein Date: Thu, 6 May 2021 02:09:54 -0400 Subject: [PATCH 4/5] Fix astyle CI failures --- .../include/drivers/interfaces/InterfaceCAN.h | 48 +++++++++---------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/drivers/include/drivers/interfaces/InterfaceCAN.h b/drivers/include/drivers/interfaces/InterfaceCAN.h index 6429b00ede2..c737f382137 100644 --- a/drivers/include/drivers/interfaces/InterfaceCAN.h +++ b/drivers/include/drivers/interfaces/InterfaceCAN.h @@ -118,35 +118,35 @@ namespace interface { /* Having this as a struct allows interface::CAN and/or mbed::CAN to inherit the enums */ struct can { -enum Mode { - Reset = 0, - Normal, - Silent, - LocalTest, - GlobalTest, - SilentTest -}; - -enum IrqType { - RxIrq = 0, - TxIrq, - EwIrq, - DoIrq, - WuIrq, - EpIrq, - AlIrq, - BeIrq, - IdIrq, - - IrqCnt -}; + enum Mode { + Reset = 0, + Normal, + Silent, + LocalTest, + GlobalTest, + SilentTest + }; + + enum IrqType { + RxIrq = 0, + TxIrq, + EwIrq, + DoIrq, + WuIrq, + EpIrq, + AlIrq, + BeIrq, + IdIrq, + + IrqCnt + }; // Prevent slicing and user creation of base class. protected: can() = default; ~can() = default; - can(const can&) = default; - can& operator=(const can&) = default; + can(const can &) = default; + can &operator=(const can &) = default; }; From d39a4168ec0a7d440959ebd16f2b439bd76b2ced Mon Sep 17 00:00:00 2001 From: George Beckstein Date: Fri, 7 May 2021 21:02:34 -0400 Subject: [PATCH 5/5] Replace NonCopyable in CAN inheritance. This commit moves the deletion of copy constructor and copy assignment operators to the `mbed::interface::can` class, where both `mbed::CAN` and `mbed::interface::CAN` inherit enum types from. This allows `NonCopyable` to be removed from the inheritance list. --- drivers/include/drivers/CAN.h | 5 ++--- drivers/include/drivers/interfaces/InterfaceCAN.h | 8 ++++++-- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/drivers/include/drivers/CAN.h b/drivers/include/drivers/CAN.h index c5621f73133..be404f8955d 100644 --- a/drivers/include/drivers/CAN.h +++ b/drivers/include/drivers/CAN.h @@ -25,7 +25,6 @@ #include "hal/can_api.h" #include "platform/Callback.h" #include "platform/PlatformMutex.h" -#include "platform/NonCopyable.h" namespace mbed { @@ -39,9 +38,9 @@ namespace mbed { */ class CAN #ifdef FEATURE_EXPERIMENTAL_API - final : public interface::CAN, private NonCopyable + final : public interface::CAN #else - : private NonCopyable, public interface::can + : public interface::can #endif { diff --git a/drivers/include/drivers/interfaces/InterfaceCAN.h b/drivers/include/drivers/interfaces/InterfaceCAN.h index c737f382137..54cf0d2c963 100644 --- a/drivers/include/drivers/interfaces/InterfaceCAN.h +++ b/drivers/include/drivers/interfaces/InterfaceCAN.h @@ -145,8 +145,12 @@ struct can { protected: can() = default; ~can() = default; - can(const can &) = default; - can &operator=(const can &) = default; + +public: + + /* Copy constructor and copy assignment operators will be deleted in subclasses as well */ + can(const can &) = delete; + can &operator=(const can &) = delete; };