Skip to content

Commit

Permalink
fix: Addressed a lot of code-smells sonarcloud reported.
Browse files Browse the repository at this point in the history
  • Loading branch information
chrisdutz committed Jul 22, 2023
1 parent 4c4155e commit 3c2978d
Show file tree
Hide file tree
Showing 16 changed files with 278 additions and 268 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,11 @@

public class SDODownloadConversation extends CANOpenConversationBase {

private final CANConversation delegate;
private final IndexAddress indexAddress;
private final byte[] data;

public SDODownloadConversation(CANConversation delegate, int nodeId, int answerNodeId, IndexAddress indexAddress, PlcValue value, CANOpenDataType type) {
super(delegate, nodeId, answerNodeId);
this.delegate = delegate;
this.indexAddress = indexAddress;

try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -819,7 +819,7 @@ private PlcValue parsePlcValue(EipTag tag, ByteBuf data, CIPDataTypeCode type) {
// TODO: return as type STRUCT with structuredType to let user
// apps/progs handle it.
}
// TODO: This will fall-though to "default"
return null;
}
default:
return null;
Expand Down Expand Up @@ -852,7 +852,7 @@ private PlcValue parsePlcValue(EipTag tag, ByteBuf data, CIPDataTypeCode type) {
else {
// This is a different type of STRUCTURED data
}
// TODO: This will fall-though to "default"
return null;
}
default:
return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,9 @@ public class EipTag implements PlcTag, Serializable {
private static final Pattern ADDRESS_PATTERN =
Pattern.compile("^(?<tag>[%a-zA-Z_.0-9]+\\[?[0-9]*]?):?(?<dataType>[A-Z]*):?(?<elementNb>[0-9]*)");

private static final String TAG = "tag";
private static final String ELEMENTS = "elementNb";
private static final String TYPE = "dataType";

private static final String GROUP_NAME_TAG = "tag";
private static final String GROUP_NAME_GROUP_NAME_ELEMENTS = "elementNb";
private static final String GROUP_NAME_TYPE = "dataType";

private final String tag;
private CIPDataTypeCode type;
Expand Down Expand Up @@ -111,14 +110,14 @@ public static boolean matches(String tagQuery) {
public static EipTag of(String tagString) {
Matcher matcher = ADDRESS_PATTERN.matcher(tagString);
if (matcher.matches()) {
String tag = matcher.group(TAG);
String tag = matcher.group(GROUP_NAME_TAG);
int nb = 1;
CIPDataTypeCode type;
if (!matcher.group(ELEMENTS).isEmpty()) {
nb = Integer.parseInt(matcher.group(ELEMENTS));
if (!matcher.group(GROUP_NAME_GROUP_NAME_ELEMENTS).isEmpty()) {
nb = Integer.parseInt(matcher.group(GROUP_NAME_GROUP_NAME_ELEMENTS));
}
if (!matcher.group(TYPE).isEmpty()) {
type = CIPDataTypeCode.valueOf(matcher.group(TYPE));
if (!matcher.group(GROUP_NAME_TYPE).isEmpty()) {
type = CIPDataTypeCode.valueOf(matcher.group(GROUP_NAME_TYPE));
} else {
type = CIPDataTypeCode.DINT;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,14 +74,15 @@ public EtsModel parse(File knxprojFile, String password) {
File knxMasterFile = new File(tempDir.toFile(), "knx_master.xml");
// If the file contains: <KNX xmlns="http://knx.org/xml/project/21"> it's an ETS6 file
// In all other cases, we'll treat it as ETS5
Scanner scanner = new Scanner(knxMasterFile);
String etsSchemaVersion = null;
while (scanner.hasNextLine()) {
final String curLine = scanner.nextLine();
if(curLine.contains("http://knx.org/xml/project/")) {
etsSchemaVersion = curLine.substring(curLine.indexOf("http://knx.org/xml/project/") + "http://knx.org/xml/project/".length());
etsSchemaVersion = etsSchemaVersion.substring(0, etsSchemaVersion.indexOf("\""));
break;
try(Scanner scanner = new Scanner(knxMasterFile)) {
while (scanner.hasNextLine()) {
final String curLine = scanner.nextLine();
if(curLine.contains("http://knx.org/xml/project/")) {
etsSchemaVersion = curLine.substring(curLine.indexOf("http://knx.org/xml/project/") + "http://knx.org/xml/project/".length());
etsSchemaVersion = etsSchemaVersion.substring(0, etsSchemaVersion.indexOf("\""));
break;
}
}
}
EtsFileHandler fileHandler = ("21".equals(etsSchemaVersion)) ? new Ets6FileHandler() : new Ets5FileHandler();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,20 +120,17 @@ public boolean matchesGroupAddress(GroupAddress groupAddress) {
return false;
}
// NOTE: This case fallthrough is intentional :-)
switch (getLevels()) {
case 3:
if(!WILDCARD.equals(getMiddleGroup()) && !getMiddleGroup().equals(otherAddress.getMiddleGroup())) {
return false;
}
case 2:
if(!WILDCARD.equals(getSubGroup()) && !getSubGroup().equals(otherAddress.getSubGroup())) {
return false;
}
case 1:
return WILDCARD.equals(getMainGroup()) || getMainGroup().equals(otherAddress.getMainGroup());
default:
if(getLevels() == 3) {
if (!WILDCARD.equals(getMiddleGroup()) && !getMiddleGroup().equals(otherAddress.getMiddleGroup())) {
return false;
}
}
if(getLevels() >= 2) {
if (!WILDCARD.equals(getSubGroup()) && !getSubGroup().equals(otherAddress.getSubGroup())) {
return false;
}
}
return WILDCARD.equals(getMainGroup()) || getMainGroup().equals(otherAddress.getMainGroup());
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@

public class ProfinetProtocolLogic extends Plc4xProtocolBase<Ethernet_Frame> implements HasConfiguration<ProfinetConfiguration> {

private ProfinetDriverContext driverContext;
private ProfinetDriverContext profinetDriverContext;
private ProfinetConfiguration configuration;

private final Logger logger = LoggerFactory.getLogger(ProfinetProtocolLogic.class);
Expand All @@ -62,7 +62,7 @@ public void setDriverContext(DriverContext driverContext) {
throw new PlcRuntimeException(
"Expecting a driverContext of type ProfinetDriverContext, but got " + driverContext.getClass().getName());
}
this.driverContext = (ProfinetDriverContext) driverContext;
this.profinetDriverContext = (ProfinetDriverContext) driverContext;
}

@Override
Expand Down Expand Up @@ -95,18 +95,18 @@ public void onConnect(ConversationContext<Ethernet_Frame> context) {

// Check if we actually got the vendor-id and product-id ...
// without these, we don't know what to do with the device.
if ((driverContext.getVendorId() == 0) || (driverContext.getDeviceId() == 0)) {
if ((profinetDriverContext.getVendorId() == 0) || (profinetDriverContext.getDeviceId() == 0)) {
logger.error("Unable to determine vendor-id or product-id, closing channel...");
context.getChannel().close();
return;
}

// Look up the GSD file for this device ...
ProfinetISO15745Profile deviceProfile =
configuration.getGsdProfile(driverContext.getVendorId(), driverContext.getDeviceId());
configuration.getGsdProfile(profinetDriverContext.getVendorId(), profinetDriverContext.getDeviceId());
if (deviceProfile == null) {
logger.error("Unable to find GSD profile for device with vendor-id {} and device-id {}",
driverContext.getVendorId(), driverContext.getDeviceId());
profinetDriverContext.getVendorId(), profinetDriverContext.getDeviceId());
context.getChannel().close();
return;
}
Expand All @@ -115,11 +115,11 @@ public void onConnect(ConversationContext<Ethernet_Frame> context) {
if (configuration.dapId != null) {
for (ProfinetDeviceAccessPointItem profinetDeviceAccessPointItem : deviceProfile.getProfileBody().getApplicationProcess().getDeviceAccessPointList()) {
if(profinetDeviceAccessPointItem.getId().equalsIgnoreCase(configuration.dapId)) {
driverContext.setDapId(profinetDeviceAccessPointItem.getId());
profinetDriverContext.setDapId(profinetDeviceAccessPointItem.getId());
break;
}
}
if(driverContext.getDapId() == null) {
if(profinetDriverContext.getDapId() == null) {
logger.error("Couldn't find requested device access points (DAP): {}", configuration.dapId);
context.getChannel().close();
}
Expand Down Expand Up @@ -151,7 +151,7 @@ else if(deviceProfile.getProfileBody().getApplicationProcess().getDeviceAccessPo
// Try to read the RealIdentificationData ...
RawSocketChannel pnChannel = ((RawSocketChannel) context.getChannel());
CompletableFuture<PnIoCm_Block_RealIdentificationData> future1 =
PnDcpPacketFactory.sendRealIdentificationDataRequest(context, pnChannel, driverContext);
PnDcpPacketFactory.sendRealIdentificationDataRequest(context, pnChannel, profinetDriverContext);
future1.whenComplete((realIdentificationData, throwable1) -> {
if(throwable1 != null) {
logger.error("Unable to detect device access point, closing channel...", throwable1);
Expand Down Expand Up @@ -190,12 +190,12 @@ else if(deviceProfile.getProfileBody().getApplicationProcess().getDeviceAccessPo
}
long moduleIdentNumber = Long.parseLong(moduleIdentNumberStr, 16);
if(moduleIdentNumber == dapModuleIdentificationNumber) {
driverContext.setDap(curDap);
profinetDriverContext.setDap(curDap);
break;
}
}
// Abort, if we weren't able to detect a DAP.
if(driverContext.getDap() == null) {
if(profinetDriverContext.getDap() == null) {
logger.error("Unable to auto-detect the device access point, closing channel...");
context.getChannel().close();
return;
Expand Down Expand Up @@ -259,8 +259,8 @@ else if(deviceProfile.getProfileBody().getApplicationProcess().getDeviceAccessPo
}
}
}
driverContext.setModuleIndex(moduleIndex);
driverContext.setSubmoduleIndex(submoduleIndex);
profinetDriverContext.setModuleIndex(moduleIndex);
profinetDriverContext.setSubmoduleIndex(submoduleIndex);

context.fireConnected();
});
Expand Down Expand Up @@ -320,7 +320,7 @@ public CompletableFuture<PlcBrowseResponse> browse(PlcBrowseRequest browseReques
Map<String, List<PlcBrowseItem>> values = new HashMap<>();
for (String queryName : browseRequest.getQueryNames()) {
List<PlcBrowseItem> items = new ArrayList<>();
for(Map.Entry<Integer, Map<Integer, ProfinetVirtualSubmoduleItem>> slotEntry : driverContext.getSubmoduleIndex().entrySet()) {
for(Map.Entry<Integer, Map<Integer, ProfinetVirtualSubmoduleItem>> slotEntry : profinetDriverContext.getSubmoduleIndex().entrySet()) {
int slot = slotEntry.getKey();
for(Map.Entry<Integer, ProfinetVirtualSubmoduleItem> subslotEntry: slotEntry.getValue().entrySet()) {
int subslot = subslotEntry.getKey();
Expand Down Expand Up @@ -369,7 +369,7 @@ public CompletableFuture<PlcBrowseResponse> browse(PlcBrowseRequest browseReques
@Override
public CompletableFuture<PlcSubscriptionResponse> subscribe(PlcSubscriptionRequest subscriptionRequest) {
// When subscribing, we actually set up the PN IO Application Relation and make the remote device start sending data.
if (driverContext.getDap() == null) {
if (profinetDriverContext.getDap() == null) {
return CompletableFuture.failedFuture(new PlcConnectionException("DAP not set"));
}

Expand Down Expand Up @@ -420,12 +420,12 @@ public CompletableFuture<PlcSubscriptionResponse> subscribe(PlcSubscriptionReque
int subslotNumber = subslotEntry.getKey();
Map<ProfinetTag.Direction, Map<Integer, ProfinetTag>> direction = subslotEntry.getValue();

int iocsLength = driverContext.getSubmoduleIndex().get(slotNumber).get(subslotNumber).getIoData().getIocsLength();
int iocsLength = profinetDriverContext.getSubmoduleIndex().get(slotNumber).get(subslotNumber).getIoData().getIocsLength();
// The default is 1
if(iocsLength == 0) {
iocsLength = 1;
}
int iopsLength = driverContext.getSubmoduleIndex().get(slotNumber).get(subslotNumber).getIoData().getIopsLength();
int iopsLength = profinetDriverContext.getSubmoduleIndex().get(slotNumber).get(subslotNumber).getIoData().getIopsLength();
// The default is 1
if(iopsLength == 0) {
iopsLength = 1;
Expand Down Expand Up @@ -478,12 +478,12 @@ public CompletableFuture<PlcSubscriptionResponse> subscribe(PlcSubscriptionReque
blocks.add(new PnIoCm_Block_ArReq(
ProfinetDriverContext.BLOCK_VERSION_HIGH, ProfinetDriverContext.BLOCK_VERSION_LOW,
PnIoCm_ArType.IO_CONTROLLER,
driverContext.generateUuid(),
driverContext.getSessionKey(),
profinetDriverContext.generateUuid(),
profinetDriverContext.getSessionKey(),
localMacAddress,
driverContext.getCmInitiatorObjectUuid(),
profinetDriverContext.getCmInitiatorObjectUuid(),
false,
driverContext.isNonLegacyStartupMode(),
profinetDriverContext.isNonLegacyStartupMode(),
false,
false,
PnIoCm_CompanionArType.SINGLE_AR,
Expand All @@ -506,14 +506,14 @@ public CompletableFuture<PlcSubscriptionResponse> subscribe(PlcSubscriptionReque
false,
PnIoCm_RtClass.RT_CLASS_2,
ProfinetDriverContext.DEFAULT_IO_DATA_SIZE,
driverContext.getAndIncrementIdentification(),
driverContext.getSendClockFactor(),
driverContext.getReductionRatio(),
profinetDriverContext.getAndIncrementIdentification(),
profinetDriverContext.getSendClockFactor(),
profinetDriverContext.getReductionRatio(),
1,
0,
0xffffffffL,
driverContext.getWatchdogFactor(),
driverContext.getDataHoldFactor(),
profinetDriverContext.getWatchdogFactor(),
profinetDriverContext.getDataHoldFactor(),
0xC000,
ProfinetDriverContext.DEFAULT_EMPTY_MAC_ADDRESS,
Collections.singletonList(
Expand All @@ -533,14 +533,14 @@ public CompletableFuture<PlcSubscriptionResponse> subscribe(PlcSubscriptionReque
false,
PnIoCm_RtClass.RT_CLASS_2,
ProfinetDriverContext.DEFAULT_IO_DATA_SIZE,
driverContext.getAndIncrementIdentification(),
driverContext.getSendClockFactor(),
driverContext.getReductionRatio(),
profinetDriverContext.getAndIncrementIdentification(),
profinetDriverContext.getSendClockFactor(),
profinetDriverContext.getReductionRatio(),
1,
0,
0xffffffffL,
driverContext.getWatchdogFactor(),
driverContext.getDataHoldFactor(),
profinetDriverContext.getWatchdogFactor(),
profinetDriverContext.getDataHoldFactor(),
0xC000,
ProfinetDriverContext.DEFAULT_EMPTY_MAC_ADDRESS,
Collections.singletonList(
Expand All @@ -567,9 +567,9 @@ public CompletableFuture<PlcSubscriptionResponse> subscribe(PlcSubscriptionReque
DceRpc_PacketType.WORKING,
false, false, false,
IntegerEncoding.BIG_ENDIAN, CharacterEncoding.ASCII, FloatingPointEncoding.IEEE,
new DceRpc_ObjectUuid((byte) 0x00, (short) 0x0001, Integer.decode("0x" + driverContext.getDeviceId()), Integer.decode("0x" + driverContext.getVendorId())),
new DceRpc_ObjectUuid((byte) 0x00, (short) 0x0001, Integer.decode("0x" + profinetDriverContext.getDeviceId()), Integer.decode("0x" + profinetDriverContext.getVendorId())),
new DceRpc_InterfaceUuid_DeviceInterface(),
driverContext.getActivityUuid(),
profinetDriverContext.getActivityUuid(),
0L, 0L,
DceRpc_Operation.CONNECT,
(short) 0,
Expand All @@ -584,8 +584,8 @@ public CompletableFuture<PlcSubscriptionResponse> subscribe(PlcSubscriptionReque
(short) 64,
new IpAddress(localAddress.getAddress().getAddress()),
new IpAddress(remoteAddress.getAddress().getAddress()),
driverContext.getLocalPort(),
driverContext.getRemotePortImplicitCommunication(),
profinetDriverContext.getLocalPort(),
profinetDriverContext.getRemotePortImplicitCommunication(),
packet
);
Ethernet_Frame ethernetFrame = new Ethernet_Frame(
Expand All @@ -607,13 +607,13 @@ protected void extractBlockInfo(List<PnDcp_Block> blocks) {
if (blockMap.containsKey(ProfinetDiscoverer.DEVICE_TYPE_NAME)) {
PnDcp_Block_DevicePropertiesDeviceVendor block =
(PnDcp_Block_DevicePropertiesDeviceVendor) blockMap.get(ProfinetDiscoverer.DEVICE_TYPE_NAME);
driverContext.setDeviceType(new String(block.getDeviceVendorValue()));
profinetDriverContext.setDeviceType(new String(block.getDeviceVendorValue()));
}

if (blockMap.containsKey(ProfinetDiscoverer.DEVICE_NAME_OF_STATION)) {
PnDcp_Block_DevicePropertiesNameOfStation block =
(PnDcp_Block_DevicePropertiesNameOfStation) blockMap.get(ProfinetDiscoverer.DEVICE_NAME_OF_STATION);
driverContext.setDeviceName(new String(block.getNameOfStation()));
profinetDriverContext.setDeviceName(new String(block.getNameOfStation()));
}

if (blockMap.containsKey(ProfinetDiscoverer.DEVICE_ROLE)) {
Expand All @@ -632,14 +632,14 @@ protected void extractBlockInfo(List<PnDcp_Block> blocks) {
if (block.getPnioDevice()) {
roles.add("DEVICE");
}
driverContext.setRoles(roles);
profinetDriverContext.setRoles(roles);
}

if (blockMap.containsKey(ProfinetDiscoverer.DEVICE_ID)) {
PnDcp_Block_DevicePropertiesDeviceId block =
(PnDcp_Block_DevicePropertiesDeviceId) blockMap.get(ProfinetDiscoverer.DEVICE_ID);
driverContext.setVendorId(block.getVendorId());
driverContext.setDeviceId(block.getDeviceId());
profinetDriverContext.setVendorId(block.getVendorId());
profinetDriverContext.setDeviceId(block.getDeviceId());
}
}

Expand Down
Loading

0 comments on commit 3c2978d

Please sign in to comment.