Skip to content

Commit 67de368

Browse files
jsouterjsouter
committed
use lock guards on Systec handle map access
small cleanups
1 parent a32177a commit 67de368

2 files changed

Lines changed: 56 additions & 47 deletions

File tree

‎src/include/CanVendorSystec.h‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,10 @@ struct CanVendorSystec : CanDevice {
4444
CanDiagnostics vendor_diagnostics() noexcept override;
4545

4646
CanReturnCode init_can_port();
47-
47+
static std::mutex m_handles_lock;
48+
49+
inline void map_module_to_handle(int module, tUcanHandle handle) { m_handleMap[module] = handle; }
50+
inline int erase_module_handle(int module) { return m_handleMap.erase(module); }
4851

4952
// TODO i don't like this too much
5053
inline static std::unordered_map<int, tUcanHandle> m_handleMap = {};

‎src/main/CanVendorSystec.cpp‎

Lines changed: 52 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@
3030
#include <iomanip>
3131
#include <string>
3232

33+
std::mutex CanVendorSystec::m_handles_lock;
34+
3335
CanVendorSystec::CanVendorSystec(const CanDeviceArguments& args)
3436
: CanDevice("systec", args) {
3537
if (!args.config.bus_name.has_value()) {
@@ -42,30 +44,26 @@ CanVendorSystec::CanVendorSystec(const CanDeviceArguments& args)
4244
m_channelNumber = handleNumber % 2;
4345
}
4446

45-
/**
46-
* We create and fill initializationParameters, to pass it to openCanPort
47-
*/
48-
/* static */ tUcanInitCanParam createInitializationParameters( unsigned int baudRate ){
49-
tUcanInitCanParam initializationParameters;
50-
initializationParameters.m_dwSize = sizeof(initializationParameters); // size of this struct
51-
initializationParameters.m_bMode = kUcanModeNormal; // normal operation mode
52-
initializationParameters.m_bBTR0 = HIBYTE( baudRate ); // baudrate
53-
initializationParameters.m_bBTR1 = LOBYTE( baudRate );
54-
initializationParameters.m_bOCR = 0x1A; // standard output
55-
initializationParameters.m_dwAMR = USBCAN_AMR_ALL; // receive all CAN messages
56-
initializationParameters.m_dwACR = USBCAN_ACR_ALL;
57-
initializationParameters.m_dwBaudrate = USBCAN_BAUDEX_USE_BTR01;
58-
initializationParameters.m_wNrOfRxBufferEntries = USBCAN_DEFAULT_BUFFER_ENTRIES;
59-
initializationParameters.m_wNrOfTxBufferEntries = USBCAN_DEFAULT_BUFFER_ENTRIES;
60-
61-
return ( initializationParameters );
62-
}
63-
47+
// TODO should we make this noexcept? how can we guarantee that?
6448
CanReturnCode CanVendorSystec::init_can_port() {
6549
BYTE systecCallReturn = USBCAN_SUCCESSFUL;
6650
tUcanHandle canModuleHandle;
6751

52+
unsigned int baudRate = USBCAN_BAUD_125kBit;
53+
switch (args().config.bitrate.value_or(0)) {
54+
case 50000: baudRate = USBCAN_BAUD_50kBit; break;
55+
case 100000: baudRate = USBCAN_BAUD_100kBit; break;
56+
case 125000: baudRate = USBCAN_BAUD_125kBit; break;
57+
case 250000: baudRate = USBCAN_BAUD_250kBit; break;
58+
case 500000: baudRate = USBCAN_BAUD_500kBit; break;
59+
case 1000000: baudRate = USBCAN_BAUD_1MBit; break;
60+
default: {
61+
LOG(Log::WRN, CanLogIt::h()) << "baud rate illegal, taking default 125000 [" << baudRate << "]";
62+
}
63+
}
64+
6865
// check if USB-CANmodul already is initialized
66+
std::lock_guard<std::mutex> guard(CanVendorSystec::m_handles_lock);
6967
auto pos = m_handleMap.find(m_moduleNumber);
7068
if (pos == m_handleMap.end()) { // module not in use
7169
systecCallReturn = ::UcanInitHardwareEx(&canModuleHandle, m_moduleNumber, 0, 0);
@@ -74,25 +72,22 @@ CanReturnCode CanVendorSystec::init_can_port() {
7472
::UcanDeinitHardware(canModuleHandle);
7573
return CanReturnCode::unknown_open_error;
7674
}
77-
m_handleMap[m_moduleNumber] = canModuleHandle;
75+
map_module_to_handle(m_moduleNumber, canModuleHandle);
7876
} else { // find existing handle of module
7977
canModuleHandle = pos->second;
8078
LOG(Log::WRN, CanLogIt::h()) << "trying to open a can port which is in use, reuse handle, skipping UCanDeinitHardware";
8179
}
82-
83-
unsigned int baudRate = USBCAN_BAUD_125kBit;
84-
switch (args().config.bitrate.value_or(0)) {
85-
case 50000: baudRate = USBCAN_BAUD_50kBit; break;
86-
case 100000: baudRate = USBCAN_BAUD_100kBit; break;
87-
case 125000: baudRate = USBCAN_BAUD_125kBit; break;
88-
case 250000: baudRate = USBCAN_BAUD_250kBit; break;
89-
case 500000: baudRate = USBCAN_BAUD_500kBit; break;
90-
case 1000000: baudRate = USBCAN_BAUD_1MBit; break;
91-
default: {
92-
LOG(Log::WRN, CanLogIt::h()) << "baud rate illegal, taking default 125000 [" << baudRate << "]";
93-
}
94-
}
95-
auto initializationParameters = createInitializationParameters(baudRate);
80+
tUcanInitCanParam initializationParameters;
81+
initializationParameters.m_dwSize = sizeof(initializationParameters); // size of this struct
82+
initializationParameters.m_bMode = kUcanModeNormal; // normal operation mode
83+
initializationParameters.m_bBTR0 = HIBYTE( baudRate ); // baudrate
84+
initializationParameters.m_bBTR1 = LOBYTE( baudRate );
85+
initializationParameters.m_bOCR = 0x1A; // standard output
86+
initializationParameters.m_dwAMR = USBCAN_AMR_ALL; // receive all CAN messages
87+
initializationParameters.m_dwACR = USBCAN_ACR_ALL;
88+
initializationParameters.m_dwBaudrate = USBCAN_BAUDEX_USE_BTR01;
89+
initializationParameters.m_wNrOfRxBufferEntries = USBCAN_DEFAULT_BUFFER_ENTRIES;
90+
initializationParameters.m_wNrOfTxBufferEntries = USBCAN_DEFAULT_BUFFER_ENTRIES;
9691

9792
systecCallReturn = ::UcanInitCanEx2(canModuleHandle, m_channelNumber, &initializationParameters);
9893
if ( systecCallReturn != USBCAN_SUCCESSFUL ) {
@@ -109,6 +104,7 @@ CanReturnCode CanVendorSystec::init_can_port() {
109104
CanReturnCode CanVendorSystec::vendor_open() noexcept {
110105

111106
auto returnCode = init_can_port();
107+
if (returnCode != CanReturnCode::success) return returnCode;
112108

113109
// TODO set time since opened equivalent...
114110
// m_statistics.setTimeSinceOpened();
@@ -118,14 +114,16 @@ CanReturnCode CanVendorSystec::vendor_open() noexcept {
118114

119115
if (NULL == m_hReceiveThread) {
120116
LOG(Log::ERR, CanLogIt::h()) << "Error creating the canScanControl thread.";
121-
return CanReturnCode::unknown_open_error;
117+
return CanReturnCode::internal_api_error;
122118
}
123119

124120
return returnCode;
125121
}
126122

127123
CanReturnCode CanVendorSystec::vendor_close() noexcept {
128124
// TODO what if the return code is not success?
125+
std::lock_guard<std::mutex> guard(CanVendorSystec::m_handles_lock);
126+
erase_module_handle(m_moduleNumber);
129127
m_CanScanThreadShutdownFlag = false;
130128
DWORD result = WaitForSingleObject(m_hReceiveThread, INFINITE); //Shut down can scan thread
131129
UcanDeinitCanEx (m_UcanHandle, (BYTE)m_channelNumber);
@@ -138,9 +136,7 @@ CanReturnCode CanVendorSystec::vendor_send(const CanFrame& frame) noexcept {
138136
bool rtr = frame.is_remote_request();
139137
uint32_t len = frame.length();
140138
char *message = frame.message().data();
141-
short cobID = frame.id(); // is this the same as cobid? i don't know...
142-
143-
// TODO we can't just use message as it's now a vector of chars not a char*
139+
short cobID = frame.id();
144140

145141
LOG(Log::DBG, CanLogIt::h()) << "Sending message: [" << ( message == 0 ? "" : (const char *) message) << "], cobID: [" << cobID << "], Message Length: [" << static_cast<int>(len) << "]";
146142

@@ -172,17 +168,27 @@ CanReturnCode CanVendorSystec::vendor_send(const CanFrame& frame) noexcept {
172168
if (Status != USBCAN_SUCCESSFUL) {
173169
LOG(Log::ERR, CanLogIt::h()) << "There was a problem when sending a message.";
174170

175-
// for now, just always reconnect on a failed send.
176-
vendor_close();
177-
vendor_open();
178-
179-
return CanReturnCode::unknown_send_error;
180-
} else {
171+
// for now, just always reconnect on a failed send.
172+
vendor_close(); // TODO maybe we just call close instead of vendor_close
173+
// see how CanVendorSocketCanSystec does reconnects, it intercepts the receiver function and wraps it
174+
vendor_open();
175+
176+
switch (Status) {
177+
case USBCAN_ERR_MAXINSTANCES: return CanReturnCode::too_many_connections;
178+
case USBCAN_ERR_ILLHANDLE: return CanReturnCode::disconnected;
179+
case USBCAN_ERR_CANNOTINIT: return CanReturnCode::unknown_open_error; // maybe disconnected would be better
180+
case USBCAN_ERR_DLL_TXFULL: return CanReturnCode::tx_buffer_overflow;
181+
case USBCAN_ERR_ILLPARAM: // fallthrough
182+
case USBCAN_ERR_ILLHW:
183+
case USBCAN_ERR_ILLCHANNEL:
184+
case USBCAN_WARN_FW_TXOVERRUN: // TODO should warnings return an error?
185+
case USBCAN_WARN_TXLIMIT:
186+
default: return CanReturnCode::unknown_send_error;
187+
}
181188
// m_statistics.onTransmit( canMsgToBeSent.m_bDLC );
182189
// m_statistics.setTimeSinceTransmitted();
183-
return CanReturnCode::success;
184190
}
185-
// return sendErrorCode(Status);
191+
return CanReturnCode::success;
186192
};
187193

188194
CanDiagnostics CanVendorSystec::vendor_diagnostics() noexcept {

0 commit comments

Comments
 (0)