Skip to content
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

Current measurement #260

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

Conversation

tanner87661
Copy link

Change overview

  1. Changes to the command reference
  • response removed from the request response as suggested in source code note
  • recycling of for event driven reporting of current readings. Format: with 0 for main track, 1 for prog track, VALUE is getCurrentmA(). <a could be changed to something else since <a is already used for incoming commands
  • Introduction of command. MAINCURRENT and PROGCURRENT .control how current measurement is done for main and prog track. If 0, current measurement for the track is OFF. If set to 1, current readings are broadcasted using , leading to 2 messages per 1/10 of a second. If set to > 1, current measurement is internal, with the number indicating the number of history values that are considered for rolling average. Example: <A 30 0> sets the current measurement for MAIN to internal with a buffer size of 30 values (about 3 secs), current measurement on PROG is OFF
  • Introduction of <c 0> command, a modified form of . It reports current from the internal measurement using the new getCurrentRMS() function. Same format as , just two values for current reported based on the rolling RMS calculation
  1. Measurement methodology
    a) Internal
    After each current measurement, the getCurrentmA() value is squared and added to a rolling average. When getCurrentRMS() is called, the rolling average is divided by buffer size and rounded sqrt is reported. This is the current in mA. Advantage is the calculation is done in DCC EX. Disadvantage is that due to the average calculation reaction of going down to zero is slow
    b) broadcast
    After each current measurement, the getCurrentmA() result is sent to the serial interface. Processing of RMS calculation is done on the host computer where more memory is available to do a ring buffer for each track for the square values, leading to faster phase out if zero current. This is my preferred solution for the RedHat, would also work for JMRI

Copy link
Contributor

@Asbelos Asbelos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use of outstream passed into dccwaveform will corrupt wifi buffers if the command is given from a wifi connected jmri or throttle. (Same problem for Ethernet)
Asynchronous additions to wifi/ethernet streams require special handling as the stream is shared between multiple clients and may or may not be in a client transaction at the time the message is issued.
For examples see how prog track commands use callback mechanism and stash the client id.

@tanner87661
Copy link
Author

Please help me to understand. What is async about those calls? They are made from the loop function, similar like responses in DCCExParser. outStream is just a pointer to the stream that is also used in DCCExParser within the same loop system. That way I can send feedback right when analogRead is done. Alternatively I could set a flag and do a separate reporting function and call it from SerialManager.loop2 with serial as parameter. Also this, in my understanding, would be in sync with everything else. What am I missing?

@Asbelos
Copy link
Contributor

Asbelos commented Oct 5, 2022

What you are missing is that the stream passed to parse is only valid for the duration of the parse call... sometimes its a serial (in which case your code would work) and sometimes its a ring buffer ready prepared to return output to the wifi/ethernet caller. Most commands reply immediately without caring but things that want to reply later (eg a cv read) need to re-prepare the buffer to the correct client before writing.

The prog track commands have to follow due process to ensure that the reply goes to the original caller AND that only one caller can be asking!

The alternative is to create a broadcast function like those in CommandDistributor and send the current to all non-withrottle clients.

@tanner87661
Copy link
Author

Ok, I added broadcastCurrent. After reviewing the other broadcast functions, this seemed to me like the most logical approach.
I also changed the A command to only accept 0 and 1 to configure the return messages.
I just commented out the code for internal calculation, so we can get back to it later and do an integer version.

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.

None yet

2 participants