{"action":"created","comment":{"url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/pulls/comments/58359943","id":58359943,"diff_hunk":"@@ -24,25 +25,46 @@\n import org.slf4j.LoggerFactory;\n import org.springframework.stereotype.Component;\n \n+import com.alliander.osgp.dto.valueobjects.FirmwareVersionDto;\n+\n @Component\n-public class GetFirmwareVersionCommandExecutor implements CommandExecutor<Void, String> {\n+public class GetFirmwareVersionCommandExecutor implements CommandExecutor<Void, List<FirmwareVersionDto>> {\n \n     private static final Logger LOGGER = LoggerFactory.getLogger(GetFirmwareVersionCommandExecutor.class);\n \n+    private static final String COMMUNICATION_MODULE_ACTIVE_FIRMWARE = \"COMMUNICATION_MODULE_ACTIVE_FIRMWARE\";\n+    private static final String MODULE_ACTIVE_FIRMWARE = \"MODULE_ACTIVE_FIRMWARE\";\n+    private static final String ACTIVE_FIRMWARE = \"ACTIVE_FIRMWARE\";\n+\n     private static final int CLASS_ID = 1;\n-    private static final ObisCode OBIS_CODE = new ObisCode(\"1.0.0.2.0.255\");\n-    private static final int ATTRIBUTE_ID = 2;\n+    private static final ObisCode OBIS_CODE_ACTIVE_FIRMWARE_VERSION = new ObisCode(\"1.0.0.2.0.255\");\n+    private static final ObisCode OBIS_CODE_MODULE_ACTIVE_FIRMWARE_VERSION = new ObisCode(\"1.1.0.2.0.255\");\n+    private static final ObisCode OBIS_CODE_COMMUNICATION_MODULE_ACTIVE_FIRMWARE_VERSION = new ObisCode(\"1.2.0.2.0.255\");\n \n+    private static final int ATTRIBUTE_ID = 2;\n \n     @Override\n-    public String execute(final ClientConnection conn, final DlmsDevice device, final Void useless)\n+    public List<FirmwareVersionDto> execute(final ClientConnection conn, final DlmsDevice device, final Void useless)\n             throws ProtocolAdapterException {\n \n+        final List<FirmwareVersionDto> resultList = new ArrayList<>();\n+        resultList.add(new FirmwareVersionDto(ACTIVE_FIRMWARE, this.retrieveFirmwareData(conn,\n+                OBIS_CODE_ACTIVE_FIRMWARE_VERSION)));\n+        resultList.add(new FirmwareVersionDto(MODULE_ACTIVE_FIRMWARE, this.retrieveFirmwareData(conn,\n+                OBIS_CODE_MODULE_ACTIVE_FIRMWARE_VERSION)));\n+        resultList.add(new FirmwareVersionDto(COMMUNICATION_MODULE_ACTIVE_FIRMWARE, this.retrieveFirmwareData(conn,\n+                OBIS_CODE_COMMUNICATION_MODULE_ACTIVE_FIRMWARE_VERSION)));\n+\n+        return resultList;\n+    }\n+\n+    private String retrieveFirmwareData(final ClientConnection conn, final ObisCode obisCode)\n+            throws ProtocolAdapterException {","path":"osgp-protocol-adapter-dlms/src/main/java/org/osgp/adapter/protocol/dlms/domain/commands/GetFirmwareVersionCommandExecutor.java","position":50,"original_position":50,"commit_id":"8c07a0ba9ace1de02c07e2bd5a3648a2772d76fb","original_commit_id":"8c07a0ba9ace1de02c07e2bd5a3648a2772d76fb","user":{"login":"bvdzwet","id":14938953,"avatar_url":"https://avatars.githubusercontent.com/u/14938953?v=3","gravatar_id":"","url":"https://api.github.com/users/bvdzwet","html_url":"https://github.com/bvdzwet","followers_url":"https://api.github.com/users/bvdzwet/followers","following_url":"https://api.github.com/users/bvdzwet/following{/other_user}","gists_url":"https://api.github.com/users/bvdzwet/gists{/gist_id}","starred_url":"https://api.github.com/users/bvdzwet/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/bvdzwet/subscriptions","organizations_url":"https://api.github.com/users/bvdzwet/orgs","repos_url":"https://api.github.com/users/bvdzwet/repos","events_url":"https://api.github.com/users/bvdzwet/events{/privacy}","received_events_url":"https://api.github.com/users/bvdzwet/received_events","type":"User","site_admin":false},"body":"You could consider retrieving the firmware versions in a get-with-list call to save a couple of bytes in the communication.\r\n\r\nAnother thing I notice in the code (as it already existed before the changes regarding the multiple firmware versions) is retrieving the results, handling exceptions and interpreting result data.\r\nI think we could remove duplication on these issues by calling methods from the `DlmsHelperService` instead.\r\nMethods like `getAndCheck`, `getWithList` and `readString` look like candidates to replace some of the code fragments used in the command executor now.\r\nThe `DlmsHelperService` may need to be improved in handling exceptions, based on the difference in handling `ConnectionException`s in the command executor versus the implementation in `getWithList`.","created_at":"2016-04-04T11:10:57Z","updated_at":"2016-04-04T11:10:57Z","html_url":"https://github.com/OSGP/Protocol-Adapter-DLMS/pull/95#discussion_r58359943","pull_request_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/pulls/95","_links":{"self":{"href":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/pulls/comments/58359943"},"html":{"href":"https://github.com/OSGP/Protocol-Adapter-DLMS/pull/95#discussion_r58359943"},"pull_request":{"href":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/pulls/95"}}},"pull_request":{"url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/pulls/95","id":64799151,"html_url":"https://github.com/OSGP/Protocol-Adapter-DLMS/pull/95","diff_url":"https://github.com/OSGP/Protocol-Adapter-DLMS/pull/95.diff","patch_url":"https://github.com/OSGP/Protocol-Adapter-DLMS/pull/95.patch","issue_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/issues/95","number":95,"state":"open","locked":false,"title":"SLIM-471 firmware types","user":{"login":"rudidevries","id":5311277,"avatar_url":"https://avatars.githubusercontent.com/u/5311277?v=3","gravatar_id":"","url":"https://api.github.com/users/rudidevries","html_url":"https://github.com/rudidevries","followers_url":"https://api.github.com/users/rudidevries/followers","following_url":"https://api.github.com/users/rudidevries/following{/other_user}","gists_url":"https://api.github.com/users/rudidevries/gists{/gist_id}","starred_url":"https://api.github.com/users/rudidevries/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/rudidevries/subscriptions","organizations_url":"https://api.github.com/users/rudidevries/orgs","repos_url":"https://api.github.com/users/rudidevries/repos","events_url":"https://api.github.com/users/rudidevries/events{/privacy}","received_events_url":"https://api.github.com/users/rudidevries/received_events","type":"User","site_admin":false},"body":"Retrieve a list of firmware versions of a device.","created_at":"2016-03-31T14:27:09Z","updated_at":"2016-04-04T11:10:57Z","closed_at":null,"merged_at":null,"merge_commit_sha":"8ec8712a4644a4435d811b7fe95d930dc7864ecf","assignee":null,"milestone":null,"commits_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/pulls/95/commits","review_comments_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/pulls/95/comments","review_comment_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/pulls/comments{/number}","comments_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/issues/95/comments","statuses_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/statuses/8c07a0ba9ace1de02c07e2bd5a3648a2772d76fb","head":{"label":"OSGP:SLIM-471-Firmware-Types","ref":"SLIM-471-Firmware-Types","sha":"8c07a0ba9ace1de02c07e2bd5a3648a2772d76fb","user":{"login":"OSGP","id":11352045,"avatar_url":"https://avatars.githubusercontent.com/u/11352045?v=3","gravatar_id":"","url":"https://api.github.com/users/OSGP","html_url":"https://github.com/OSGP","followers_url":"https://api.github.com/users/OSGP/followers","following_url":"https://api.github.com/users/OSGP/following{/other_user}","gists_url":"https://api.github.com/users/OSGP/gists{/gist_id}","starred_url":"https://api.github.com/users/OSGP/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/OSGP/subscriptions","organizations_url":"https://api.github.com/users/OSGP/orgs","repos_url":"https://api.github.com/users/OSGP/repos","events_url":"https://api.github.com/users/OSGP/events{/privacy}","received_events_url":"https://api.github.com/users/OSGP/received_events","type":"Organization","site_admin":false},"repo":{"id":33675294,"name":"Protocol-Adapter-DLMS","full_name":"OSGP/Protocol-Adapter-DLMS","owner":{"login":"OSGP","id":11352045,"avatar_url":"https://avatars.githubusercontent.com/u/11352045?v=3","gravatar_id":"","url":"https://api.github.com/users/OSGP","html_url":"https://github.com/OSGP","followers_url":"https://api.github.com/users/OSGP/followers","following_url":"https://api.github.com/users/OSGP/following{/other_user}","gists_url":"https://api.github.com/users/OSGP/gists{/gist_id}","starred_url":"https://api.github.com/users/OSGP/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/OSGP/subscriptions","organizations_url":"https://api.github.com/users/OSGP/orgs","repos_url":"https://api.github.com/users/OSGP/repos","events_url":"https://api.github.com/users/OSGP/events{/privacy}","received_events_url":"https://api.github.com/users/OSGP/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/OSGP/Protocol-Adapter-DLMS","description":"Protocol adapter and device simulator for Device Language Message Specification protocol.","fork":false,"url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS","forks_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/forks","keys_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/keys{/key_id}","collaborators_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/teams","hooks_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/hooks","issue_events_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/issues/events{/number}","events_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/events","assignees_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/assignees{/user}","branches_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/branches{/branch}","tags_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/tags","blobs_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/git/refs{/sha}","trees_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/git/trees{/sha}","statuses_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/statuses/{sha}","languages_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/languages","stargazers_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/stargazers","contributors_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/contributors","subscribers_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/subscribers","subscription_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/subscription","commits_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/commits{/sha}","git_commits_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/git/commits{/sha}","comments_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/comments{/number}","issue_comment_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/issues/comments{/number}","contents_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/contents/{+path}","compare_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/compare/{base}...{head}","merges_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/merges","archive_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/downloads","issues_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/issues{/number}","pulls_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/pulls{/number}","milestones_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/milestones{/number}","notifications_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/labels{/name}","releases_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/releases{/id}","deployments_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/deployments","created_at":"2015-04-09T14:59:22Z","updated_at":"2016-01-13T08:54:07Z","pushed_at":"2016-04-01T13:39:21Z","git_url":"git://github.com/OSGP/Protocol-Adapter-DLMS.git","ssh_url":"git@github.com:OSGP/Protocol-Adapter-DLMS.git","clone_url":"https://github.com/OSGP/Protocol-Adapter-DLMS.git","svn_url":"https://github.com/OSGP/Protocol-Adapter-DLMS","homepage":"","size":8273,"stargazers_count":0,"watchers_count":0,"language":"Java","has_issues":true,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":1,"mirror_url":null,"open_issues_count":9,"forks":1,"open_issues":9,"watchers":0,"default_branch":"development"}},"base":{"label":"OSGP:development","ref":"development","sha":"f3c043fbf4e3c4426df7313f6ded5ec8e5137377","user":{"login":"OSGP","id":11352045,"avatar_url":"https://avatars.githubusercontent.com/u/11352045?v=3","gravatar_id":"","url":"https://api.github.com/users/OSGP","html_url":"https://github.com/OSGP","followers_url":"https://api.github.com/users/OSGP/followers","following_url":"https://api.github.com/users/OSGP/following{/other_user}","gists_url":"https://api.github.com/users/OSGP/gists{/gist_id}","starred_url":"https://api.github.com/users/OSGP/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/OSGP/subscriptions","organizations_url":"https://api.github.com/users/OSGP/orgs","repos_url":"https://api.github.com/users/OSGP/repos","events_url":"https://api.github.com/users/OSGP/events{/privacy}","received_events_url":"https://api.github.com/users/OSGP/received_events","type":"Organization","site_admin":false},"repo":{"id":33675294,"name":"Protocol-Adapter-DLMS","full_name":"OSGP/Protocol-Adapter-DLMS","owner":{"login":"OSGP","id":11352045,"avatar_url":"https://avatars.githubusercontent.com/u/11352045?v=3","gravatar_id":"","url":"https://api.github.com/users/OSGP","html_url":"https://github.com/OSGP","followers_url":"https://api.github.com/users/OSGP/followers","following_url":"https://api.github.com/users/OSGP/following{/other_user}","gists_url":"https://api.github.com/users/OSGP/gists{/gist_id}","starred_url":"https://api.github.com/users/OSGP/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/OSGP/subscriptions","organizations_url":"https://api.github.com/users/OSGP/orgs","repos_url":"https://api.github.com/users/OSGP/repos","events_url":"https://api.github.com/users/OSGP/events{/privacy}","received_events_url":"https://api.github.com/users/OSGP/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/OSGP/Protocol-Adapter-DLMS","description":"Protocol adapter and device simulator for Device Language Message Specification protocol.","fork":false,"url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS","forks_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/forks","keys_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/keys{/key_id}","collaborators_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/teams","hooks_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/hooks","issue_events_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/issues/events{/number}","events_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/events","assignees_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/assignees{/user}","branches_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/branches{/branch}","tags_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/tags","blobs_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/git/refs{/sha}","trees_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/git/trees{/sha}","statuses_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/statuses/{sha}","languages_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/languages","stargazers_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/stargazers","contributors_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/contributors","subscribers_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/subscribers","subscription_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/subscription","commits_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/commits{/sha}","git_commits_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/git/commits{/sha}","comments_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/comments{/number}","issue_comment_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/issues/comments{/number}","contents_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/contents/{+path}","compare_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/compare/{base}...{head}","merges_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/merges","archive_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/downloads","issues_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/issues{/number}","pulls_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/pulls{/number}","milestones_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/milestones{/number}","notifications_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/labels{/name}","releases_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/releases{/id}","deployments_url":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/deployments","created_at":"2015-04-09T14:59:22Z","updated_at":"2016-01-13T08:54:07Z","pushed_at":"2016-04-01T13:39:21Z","git_url":"git://github.com/OSGP/Protocol-Adapter-DLMS.git","ssh_url":"git@github.com:OSGP/Protocol-Adapter-DLMS.git","clone_url":"https://github.com/OSGP/Protocol-Adapter-DLMS.git","svn_url":"https://github.com/OSGP/Protocol-Adapter-DLMS","homepage":"","size":8273,"stargazers_count":0,"watchers_count":0,"language":"Java","has_issues":true,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":1,"mirror_url":null,"open_issues_count":9,"forks":1,"open_issues":9,"watchers":0,"default_branch":"development"}},"_links":{"self":{"href":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/pulls/95"},"html":{"href":"https://github.com/OSGP/Protocol-Adapter-DLMS/pull/95"},"issue":{"href":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/issues/95"},"comments":{"href":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/issues/95/comments"},"review_comments":{"href":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/pulls/95/comments"},"review_comment":{"href":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/pulls/95/commits"},"statuses":{"href":"https://api.github.com/repos/OSGP/Protocol-Adapter-DLMS/statuses/8c07a0ba9ace1de02c07e2bd5a3648a2772d76fb"}}}}