Skip to content

Fixing multiple issues #36

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Fixing multiple issues #36

wants to merge 7 commits into from

Conversation

ENeMy235
Copy link

Hi Mikko,
I'm using this library for about a 7 years. I found multiple issues during this time. Some are already resolved, but here are some that are not fixed yet. I split each of them to different commit.

  • InGXAPDU there was incorrect condition in adding Calling-AP-System-Title. It should add it when ciphering is used or when one of these HLS are used (GMAC, SHA256 and ECDSA)
  • In GXSecure there is ignored len tag for KeyInfo, which is optional, so when there is no information, it would fail. Now this length tag is properly managed.
  • In GXDLMSGSMDiagnostic there was small mistake in the setting up the operator value from recieved data.

One extra change I'm suggesting is in the extraction of a code, that creates a new DLMS object into new class. It enables users to create manufacturer specific objects more easily, whitout the need of cloning the GXDLMS class (this class is changing almost everytime). I named this class GXCreateObject and placed it into objects package.

@gurux-kurumi
Copy link

Hi,

Thank you for pointing out the following issues and they are now fixed.

  • Fix GSMDiagnostic operator value is an issue and it's fixed.
  • Fix condition in Application context name to add calling-AP-System-Title]
    Your name is added to the credits when the new version is released.

I can't accept those changes:
The settings class is planned to be used internally and it can't be changed to public.
The invocation counter will be not added to settings, because there is no use for it.

I need to verify the key info and check how customers are using it at the moment.

I need to think about whether the object creation moved to its own class.

BR,
Mikko

@ENeMy235
Copy link
Author

I was using the setInvocationCounter to set the object to null, hence it disables the invocation counter check on the server side.

And I checked what I used the public settings for and it was to get the SourceSystemTitle in the initialize the connection, to check what is the real server system title in creating encrypted connections.

@ENeMy235
Copy link
Author

@gurux-kurumi I just found another issue regarding the GSM Diagnostics.
It is missing values in the GSmPacketSwitchStatus enum. This code adds new enums occording to BlueBook.

Also I think, that in this case, the use of comment is redundant. It is much more readable, when I can see all the enums without the use of comments, that say the same thing.

Here is the updated enum:
public enum GsmPacketSwitchStatus { INACTIVE, GPRS, EDGE, UMTS, HSDPA, LTE, CDMA, LTE_CAT_M1, LTE_CAT_NB1, LTE_CAT_NB2 }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants