Re: [incubator-plc4x] 02/02: Add support for Stings

[Christofer Dutz]

Hi Justin,

I think in the driver itself we haven't implemented strings at all yet. It's a 
generally supported type, but I haven't come across being used to now. Guess we 
should create JIRA issues for accessing variables of all supported types.

Chris

Outlook for Android herunterladen


From: Justin Mclean 
Sent: Wednesday, January 10, 2018 6:27:50 AM
To: dev@plc4x.apache.org
Subject: Re: [incubator-plc4x] 02/02: Add support for Stings

Hi,

And I just committed an implementation that uses a nul character to seperate 
the strings. May be a better way of doing this - feedback welcome.

Thanks,
Justin

Re: [incubator-plc4x] 02/02: Add support for Stings

[Justin Mclean]

Hi,

And I just committed an implementation that uses a nul character to seperate 
the strings. May be a better way of doing this - feedback welcome.

Thanks,
Justin

Re: [incubator-plc4x] 02/02: Add support for Stings

[Justin Mclean]

Hi,

Strings not stings :-) But there is an sting in the tail here in that it’s only 
really going to work for a single string. If multiple strings are passed I 
think we would need to come up with some sort of terminating character so that 
the byte array can be broken up into multiple strings.

We could go with the tradition “NUL” character i.e. value if 0. Any other ideas?

Thanks,
Justin

Re: Using Github and Pull Requests

[Justin Mclean]

Hi,

> Note if PLC4X were setup to have its GitHub repo as its main repo, not this 
> non-writeable mirror thing, merging a PR would involve a committer just 
> pushing the PR’s merge button.  Also we could use Github Issues.

With the current set up we can do both those things i.e. merge PRs and use 
issue if we want.

I’m all for forks for larger changes - one issue is that longed lived forks / 
PR can run into merge issues. It generally why git flow style workflow is now 
seen as a poor idea.

On the subject to visibility I think pushes provide more that PRs (and you need 
to go to GitHub to see the PRs) but each to their own.

> There seem to be a lot of benefits to using the fork/PR model so I wonder why 
> we wouldn’t strongly encourage committers to use them too.  Are there good 
> arguments for not using them?  Maybe I’m just in the minority here… and so be 
> it :-)

But that being said I don’t see any reason for now using them. It does tend 
towards a more RTC (review then commit) process than a CTR (commit then review) 
process. Both are used at Apache.

Thanks,
Justin

Re: Using Github and Pull Requests

[Dale LaBossiere]

My introduction to git was really from a pre-ASF GitHub usage context so that 
very common fork/PR workflow is more familiar to me :-)  It’s a nice 
environment for collaboration.

Note if PLC4X were setup to have its GitHub repo as its main repo, not this 
non-writeable mirror thing, merging a PR would involve a committer just pushing 
the PR’s merge button.  Also we could use Github Issues.

I’ll just mention a couple of other benefits of using the Github/fork/PR flow 
in case they weren’t obvious:
you don’t clutter up the ASF-repo or GitHub mirror with personal short-lived 
feature/bugfix branches, they only exist in your fork/clone.  Hence a clone or 
pull of the ASF-repo/mirror doesn’t include them / bring them into your private 
clone.
PRs nicely group changes and comments/discussion that involve multiple commits
people can browse a feature’s/fix's WIP just by pushing your WIP to your fork 
(no need to pull into your clone.  When you create a PR (even if it's [WIP]) it 
provides additional visibility in to ongoing work.
Having everyone use the same (fork/PR) mechanism make everyone feel “equal” :-)
As you note, a committer can merge their own PR if they choose to

There seem to be a lot of benefits to using the fork/PR model so I wonder why 
we wouldn’t strongly encourage committers to use them too.  Are there good 
arguments for not using them?  Maybe I’m just in the minority here… and so be 
it :-)

— Dale

> On Jan 9, 2018, at 10:23 AM, Christofer Dutz  
> wrote:
> 
> Hi all,
> 
> I just had Infra turn on Travis support for our Github repo and it 
> immediately seems to have worked correctly :-)
> 
> So, we should discuss how we want to use Github in general. I know there are 
> projects that don’t really use GitHub especially Github PullRequests at all 
> and for example in Apache Edgent almost everything is done in Pull Requests. 
> So how do we want to do things here?
> 
> My opinion would be to utilize whatever the contributor wants. I personally 
> prefer to create a feature branch on the ASF Git and have Apache Jenkins auto 
> build it. But I think, especially for getting new people on board, Github’s 
> workflow is a valuable addition.
> 
> So how about this:
> If someone inside the team wants to work on Github, he just does it, creates 
> a pull request and merges it himself as soon as he sees the PR being fit for 
> merge. If he wants feedback he asks for feedback on the list and someone else 
> merges this as soon as the review has been done. If some outside user creates 
> a PR, we review within the team it and apply it if we see it fit of being 
> merged.
> 
> I wouldn’t make things too complicated or restrict ourselves to one set of 
> tools. I think we have setup everything to allow all sorts of paths for 
> getting code in and having it tested. Allowing all of them keeps the 
> community most open to others.
> 
> What do you think?
> 
> Chris
> 
> 

Re: Using Github and Pull Requests

[Christofer Dutz]

As Dale just responded to the test PR I created, having a prefix of “[WIP]” 
(Work in Progress) PRs should not be merged … I think that’s very reasonable :)

Chris

Am 09.01.18, 16:23 schrieb "Christofer Dutz" :

Hi all,

I just had Infra turn on Travis support for our Github repo and it 
immediately seems to have worked correctly :-)

So, we should discuss how we want to use Github in general. I know there 
are projects that don’t really use GitHub especially Github PullRequests at all 
and for example in Apache Edgent almost everything is done in Pull Requests. So 
how do we want to do things here?

My opinion would be to utilize whatever the contributor wants. I personally 
prefer to create a feature branch on the ASF Git and have Apache Jenkins auto 
build it. But I think, especially for getting new people on board, Github’s 
workflow is a valuable addition.

So how about this:
If someone inside the team wants to work on Github, he just does it, 
creates a pull request and merges it himself as soon as he sees the PR being 
fit for merge. If he wants feedback he asks for feedback on the list and 
someone else merges this as soon as the review has been done. If some outside 
user creates a PR, we review within the team it and apply it if we see it fit 
of being merged.

I wouldn’t make things too complicated or restrict ourselves to one set of 
tools. I think we have setup everything to allow all sorts of paths for getting 
code in and having it tested. Allowing all of them keeps the community most 
open to others.

What do you think?

Chris

[GitHub] chrisdutz commented on issue #2: Test travis integration

[GitBox]

chrisdutz commented on issue #2: Test travis integration
URL: https://github.com/apache/incubator-plc4x/pull/2#issuecomment-356315847
 
 
   Aaah that's a good idea ... we should do that ...
   


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

Using Github and Pull Requests

[Christofer Dutz]

Hi all,

I just had Infra turn on Travis support for our Github repo and it immediately 
seems to have worked correctly :-)

So, we should discuss how we want to use Github in general. I know there are 
projects that don’t really use GitHub especially Github PullRequests at all and 
for example in Apache Edgent almost everything is done in Pull Requests. So how 
do we want to do things here?

My opinion would be to utilize whatever the contributor wants. I personally 
prefer to create a feature branch on the ASF Git and have Apache Jenkins auto 
build it. But I think, especially for getting new people on board, Github’s 
workflow is a valuable addition.

So how about this:
If someone inside the team wants to work on Github, he just does it, creates a 
pull request and merges it himself as soon as he sees the PR being fit for 
merge. If he wants feedback he asks for feedback on the list and someone else 
merges this as soon as the review has been done. If some outside user creates a 
PR, we review within the team it and apply it if we see it fit of being merged.

I wouldn’t make things too complicated or restrict ourselves to one set of 
tools. I think we have setup everything to allow all sorts of paths for getting 
code in and having it tested. Allowing all of them keeps the community most 
open to others.

What do you think?

Chris

[GitHub] dlaboss commented on issue #2: Test travis integration

[GitBox]

dlaboss commented on issue #2: Test travis integration
URL: https://github.com/apache/incubator-plc4x/pull/2#issuecomment-356313586
 
 
   FYI Putting "[WIP]" in the PR's subject line is the usual way to convey 
"don't merge this yet" :-)


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] chrisdutz opened a new pull request #2: Test travis integration

[GitBox]

chrisdutz opened a new pull request #2: Test travis integration
URL: https://github.com/apache/incubator-plc4x/pull/2
 
 
   Just a test to check if travis is kicking in ... PLEASE, PLEASE, PLEASE 
don't merge ;-)


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

Re: First Apache PLC4X talk at a conference :-)

[Dale LaBossiere]

Looking fwd to hearing how it went.  I’m sure you’ll crush it!
— Dale

> On Jan 9, 2018, at 7:25 AM, Christofer Dutz  wrote:
> 
> Hi all,
> 
> it seems that I will be having a talk about Apache PLC4X (and Apache Edgent) 
> at this year’s Nortec conference in Hamburg in about 2 Weeks 
> (http://www.nortec-hamburg.de/en/)
> 
> Guess this will be an interesting experience as I have never had a talk on a 
> non-it conference.
> 
> Chris
> 

[GitHub] chrisdutz commented on issue #1: Warning cleanup

[GitBox]

chrisdutz commented on issue #1: Warning cleanup
URL: https://github.com/apache/incubator-plc4x/pull/1#issuecomment-356294407
 
 
   Well we should get the Travis support working first or you will be working 
without CI ... I've already setup the travis file thing, but still have to have 
Infra enable it.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

[GitHub] dlaboss commented on issue #1: Warning cleanup

[GitBox]

dlaboss commented on issue #1: Warning cleanup
URL: https://github.com/apache/incubator-plc4x/pull/1#issuecomment-356292913
 
 
   Hi Justin, I was expecting to merge it myself, but thanks :-)
   Isn't is just better to consistently work in the github 
fork/feature-branch/PR flow so as to reinforce its use?  It just makes it 
easier for folks to identify/review/comment on changes. And when build/test 
checks are integrated into the PR flow their "passing" is made readily apparent 
in the PR.


This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

First Apache PLC4X talk at a conference :-)

[Christofer Dutz]

Hi all,

it seems that I will be having a talk about Apache PLC4X (and Apache Edgent) at 
this year’s Nortec conference in Hamburg in about 2 Weeks 
(http://www.nortec-hamburg.de/en/)

Guess this will be an interesting experience as I have never had a talk on a 
non-it conference.

Chris

Git problems

[Christofer Dutz]

Seems we are having problems with our Git repo,

I couldn’t push my last changes as the push is rejected cause some remote 
pre-commit-hooks are failing. Executing “git push –v” however tells me that the 
repo seems to be out of sync and that I should contact infra … I already did 
that. Let’s hope they fix things fast ;-)

Chris

Re: [incubator-plc4x] 06/06: Sort divided by a int is always an int no need to round up with Math.ceil

[Christofer Dutz]

Yup … exactly

Chris

Am 09.01.18, 09:42 schrieb "Justin Mclean" :

Hi,

> you are correct with your assumption. However as soon as at least one 
floating point number is used in the equation, the result should be too, so it 
should be enough to make 8 an 8.0

So this what I was thinking:
(short) Math.ceil(userData.readShort() / 8.0)

Thanks,
Justin

Re: Coverage reports

[Christofer Dutz]

Hi Dale,

Unfortunately, the configuration of the coverage checks didn’t work anyway so 
they didn’t inherit a not working config. I just noticed that the rules are 
parameters of the “check” goal. So, I added an execution of that goal to the 
build. Also, I moved the configuration to a self-activating java profile so now 
they are working in the Edgent module too and there the report says:

[INFO] --- jacoco-maven-plugin:0.7.9:check (check-coverage) @ apache-edgent ---
[INFO] Loading execution data file 
/Users/christofer.dutz/Projects/Apache/PLC4X/integrations/apache-edgent/target/target/coverage-reports/jacoco-ut.exec
[INFO] Analyzed bundle 'apache-edgent' with 5 classes
[INFO] All coverage checks have been met.

Good job :-)

I however had to prevent failing coverage reports to break the build as the 
camel integration is less well tested:

[INFO] --- jacoco-maven-plugin:0.7.9:check (check-coverage) @ apache-camel ---
[INFO] Loading execution data file 
/Users/christofer.dutz/Projects/Apache/PLC4X/integrations/apache-camel/target/target/coverage-reports/jacoco-ut.exec
[INFO] Analyzed bundle 'apache-camel' with 4 classes
[WARNING] Rule violated for bundle apache-camel: instructions covered ratio is 
0.00, but expected minimum is 0.50
[WARNING] Rule violated for bundle apache-camel: classes missed count is 4, but 
expected maximum is 0
[WARNING] Coverage checks have not been met. See log for details.

It would be cool if the camel adapter could receive some tests ;-)

Chris


Am 08.01.18, 18:36 schrieb "Dale LaBossiere" :

On the topic of coverage…

The way the “integrations” are setup, they aren’t inheriting the “Fail the 
build” config setup in plc4j/pom.xml.
Is that intentional?  Should that config be pushed up into the top level 
pom, or duplicated in integrations/pom.xml (or individual integrations)?

Also it would be helpful if DEVELOPMENT.md were updated to capture the “how 
to run sonar locally” info below.

Thanks!
— Dale

> On Jan 6, 2018, at 12:32 PM, Christofer Dutz  
wrote:
> 
> Hi Justin,
> 
> well I just installed and started a local sonarqube server and executed 
this:
> 
> clean install sonar:sonar -Dsonar.host.url=http://localhost:9000/
> 
> And it worked file.
> 
> Chris
> 
> Am 06.01.18, 01:24 schrieb "Justin Mclean" :
> 
>Hi,
> 
>I've see you changed it so that running locally no longer works and 
it’s publishing to Apache builds.
> 
>That’s great but how would I test my changes and run sonar locally 
before having to commit changes?
> 
>Thanks,
>Justin
> 

Re: [incubator-plc4x] 06/06: Sort divided by a int is always an int no need to round up with Math.ceil

[Justin Mclean]

Hi,

> you are correct with your assumption. However as soon as at least one 
> floating point number is used in the equation, the result should be too, so 
> it should be enough to make 8 an 8.0

So this what I was thinking:
(short) Math.ceil(userData.readShort() / 8.0)

Thanks,
Justin

Re: [incubator-plc4x] 06/06: Sort divided by a int is always an int no need to round up with Math.ceil

[Christofer Dutz]

Hi Justin,

you are correct with your assumption. However as soon as at least one floating 
point number is used in the equation, the result should be too, so it should be 
enough to make 8 an 8.0

And no, size in bits is a Boolean and it simply provides information on if the 
size parameter is the number of bytes (false) or the number of bits (true). And 
no, if I want to read 3 consecutive bits for example, then we have to transport 
this in 1 byte, if we read 9 bits, then in 2 and so on.

Chris




Am 08.01.18, 04:24 schrieb "Justin Mclean" :

Hi,

With this change here:

> DataTransportSize dataTransportSize = 
DataTransportSize.valueOf(userData.readByte());
> short length = (dataTransportSize.isSizeInBits()) 
?
> -(short) Math.ceil(userData.readShort() / 8) 
: userData.readShort();
> +(short) (userData.readShort() / 8) : 
userData.readShort();

Math.ceil would never round up as a short divided by an init is never a 
floating point number.

Do we need to worry about non byte boundaries? i.e. is isSizeInBits always 
multiple of 8? If not the code will need to be modified as it would still be 
doing what it was doing before and sometimes missing a byte. 

Thanks,
Justin