openlcb / OpenLCB_Java

A git-managed copy of the SVN-based Java prototype implementation of OpenLCB. This implementation operates inside JMRI.
6 stars 9 forks source link

Adding Send button to EventIDs - allows testing of evetnids from CDI. #189

Closed dpharris closed 2 years ago

dpharris commented 2 years ago

Adds a 'Send' button to all events in the CDI, next to the 'Refresh' and 'Write' buttons.

balazsracz commented 2 years ago

Hey David, did you push your most recent version of code to this PR?

dpharris commented 2 years ago

Balazs --

Not yet. I was wondering if we need to add any more functionality? If not, I will clean them up (I have commented out lines, etc.).

Also, these additions required a change to OpenLCB code:

https://github.com/openlcb/OpenLCB_Java/blob/master/src/org/openlcb/cdi/swing/CdiPanel.java, and resulting openlcb.jar, and to JMRI code:

https://github.com/JMRI/JMRI/blob/master/java/src/jmri/jmrix/openlcb/swing/ClientActions.java

How should I push these, is it necessary to link them in their respective repositories?

Thanks David

On Sat, Feb 12, 2022 at 2:45 AM Balazs Racz @.***> wrote:

@.**** commented on this pull request.

In src/org/openlcb/cdi/swing/CdiPanel.java https://github.com/openlcb/OpenLCB_Java/pull/189#discussion_r805146917:

@@ -8,6 +8,8 @@ import org.openlcb.cdi.impl.ConfigRepresentation; import org.openlcb.implementations.EventTable; import org.openlcb.swing.EventIdTextField; +import org.openlcb.ProducerConsumerEventReportMessage; +import org.openlcb.*;

I don't think this import is necessary.

— Reply to this email directly, view it on GitHub https://github.com/openlcb/OpenLCB_Java/pull/189#pullrequestreview-880844940, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEDQSTPUTOUI4HES4PZKH3U2Y253ANCNFSM5MW27IKQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you authored the thread.Message ID: @.***>

balazsracz commented 2 years ago

I think what you have done so far is a good start, and it's worth to review and merge as is. We can discuss further functionality afterwards.

We have the ability to make coordinated changes in JMRI and OpenLCB. We submit and merge the openlcb change first. Then we make a release of OpenLCB. As a last step, we upgrade the JMRI dependency to the new release and in the same PR we make the JMRI change.

dpharris commented 2 years ago

Ok, I will submit my OpenLCB changes first. Then we can coordinate the JMTI change. Actually, I can submit my JMRI change in its own branch, right?

I will do that tonight or tomorrow.

David

On Sat, Feb 12, 2022 at 12:20 PM Balazs Racz @.***> wrote:

I think what you have done so far is a good start, and it's worth to review and merge as is. We can discuss further functionality afterwards.

We have the ability to make coordinated changes in JMRI and OpenLCB. We submit and merge the openlcb change first. Then we make a release of OpenLCB. As a last step, we upgrade the JMRI dependency to the new release and in the same PR we make the JMRI change.

— Reply to this email directly, view it on GitHub https://github.com/openlcb/OpenLCB_Java/pull/189#issuecomment-1037450231, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEDQSTPGFWDNZM4UIUP5NLU226JXANCNFSM5MW27IKQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you authored the thread.Message ID: @.***>

balazsracz commented 2 years ago

you should push it to this addSend branch, then I can review the code in the PR.

dpharris commented 2 years ago

Ok, will do.

David

On Sat, Feb 12, 2022 at 3:26 PM Balazs Racz @.***> wrote:

you should push it to this addSend branch, then I can review the code in the PR.

— Reply to this email directly, view it on GitHub https://github.com/openlcb/OpenLCB_Java/pull/189#issuecomment-1037570611, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEDQSQM4XIZHIAXVPVOPS3U23UAXANCNFSM5MW27IKQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you authored the thread.Message ID: @.***>

dpharris commented 2 years ago

Balazs --

I have committed my changes to my branch "morebutton", and I pushed it to openlcb_java git.

I have committed my changes to JMRI, but my push to JMRI github failed -- I do not have permission:

@.*** JMRI % git push --set-upstream origin morebutton

remote: Permission to JMRI/JMRI.git denied to dpharris.

fatal: unable to access 'https://github.com/JMRI/JMRI.git/': The requested URL returned error: 403

Here are my JMRI diffs:

@.*** JMRI % git diff master morebutton

diff --git a/java/src/jmri/jmrix/openlcb/swing/ClientActions.java b/java/src/jmri/jmrix/openlcb/swing/ClientActions.java

index 8810d80915..ff15c3fc1f 100644

--- a/java/src/jmri/jmrix/openlcb/swing/ClientActions.java

+++ b/java/src/jmri/jmrix/openlcb/swing/ClientActions.java

@@ -68,6 +68,16 @@ public class ClientActions {

             desc = null;

         }

diff --git a/lib/openlcb.jar b/lib/openlcb.jar

index ef965ba75c..8681bd4705 100644

Binary files a/lib/openlcb.jar and b/lib/openlcb.jar differ

David

On Sat, Feb 12, 2022 at 3:32 PM David Harris @.***> wrote:

Ok, will do.

David

On Sat, Feb 12, 2022 at 3:26 PM Balazs Racz @.***> wrote:

you should push it to this addSend branch, then I can review the code in the PR.

— Reply to this email directly, view it on GitHub https://github.com/openlcb/OpenLCB_Java/pull/189#issuecomment-1037570611, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEDQSQM4XIZHIAXVPVOPS3U23UAXANCNFSM5MW27IKQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you authored the thread.Message ID: @.***>

balazsracz commented 2 years ago

This PR is replaced by #194.