Hi everyone,
I am building a wireless S88 system which will deliver the feedback for my modelrailroad.
I am almost finished but I have a problem with the array which saves the data from the slaves.
The first place in the array S88DataOut[0] stays always 0 however the incoming data is 1.
Can someone help me?
Cheers,
Dylan
S88ArduinoMasterWirelessV2.ino (8.96 KB)
uint16_t S88DataOutShiftRegister1; // Shift register 1
uint16_t S88DataOutShiftRegister2; // Shift register 2
uint16_t S88DataOutShiftRegister3; // Shift register 3
uint16_t S88DataOutShiftRegister4; // Shift register 4
uint16_t S88DataOutShiftRegister5; // Shift register 5
uint16_t S88DataOutShiftRegister6; // Shift register 6
uint16_t S88DataOutShiftRegister7; // Shift register 7
uint16_t S88DataOutShiftRegister8; // Shift register 8
uint16_t S88DataOutShiftRegister9; // Shift register 9
uint16_t S88DataOutShiftRegister10; // Shift register 10
Arrays!
byte slaveID[9] = {1, 2, 3, 4, 5, 6, 7, 8};
Why do you need an array when every element has the value element position plus one?
byte S88Incomingdata[16];
uint8_t S88Outdata[160]; // S88 data, when this works completely I want to add the wireless stuff to manipulate this data array.
Why are these two arrays different types? Why are the different sizes?
The first place in the array S88DataOut[0] stays always 0 however the incoming data is 1.
How do you know this?
The code looks terribly over complicated to me.
Like PaulS has said, you can reduce the code complexity using arrays and for loops.
However, I also wonder why you seem to be reading binary data into an array, and then have loads of code to push one bit from each element in the array into other variables.
I.e I presume that the data in coming in one bit at a time.
It would be much simpler to put the incoming data straight into the final data array
I.e rather than hard coding the shift << and >>, e.g. >>15 you should use a variable based on the number position of the incoming data.
I don't have time to write an example of how to do this, but I'm sure there are plenty of libraries that have to read incoming binary data e.g virtual wire, that you could look at for inspiration
Those 2 arrays are necessary for the MiRF library since the size must be the same for the slave and the master otherwise the communication don't work...
I can't send the whole data array out in one time because my system for my modelrailroad doesn`t accept that.
Cheers Dylan
Cheers,
Dylan
Your Interrupt Service Routines (ISRs) CLOCK and LOAD are much too long. You should aim to have only two or 3 lines of code in them. It could be that you are missing something because too much time is spent in the ISR.
I wrote an Arduino program to decode DCC data. That process may be similar to what you are trying to do. I will be happy to share the code.
Can you give us a general description of how your code is intended to work as that would allow people to advise you on the best approach.
...R
Still doesn't make any sense
Those arrays are defined as
byte S88Incomingdata[16];
uint8_t S88Outdata[160]
But I can't see how S88OutData get any more than the first 16 elements filled with data e.g. here
for (int i = 0; i < 16; i++)
{
S88Outdata[i] = S88Incomingdata[i];
}
But then you have this huge bit of code
void LOAD() // Setup the S88data arrays / shift registers... Now it's ready to be send!
{
detachInterrupt(1); // Disable interrupt while doing some stuff with it.
S88DataOutShiftRegister1 = S88Outdata[0] | (S88Outdata[1] << 1) | (S88Outdata[2] << 2) | (S88Outdata[3] << 3) | (S88Outdata[4] << 4) | (S88Outdata[5] << 5) | (S88Outdata[6] << 6) | (S88Outdata[7] << 7) | (S88Outdata[8] << 8) | (S88Outdata[9] << 9) | (S88Outdata[10] << 10) | (S88Outdata[11] << 11) | (S88Outdata[12] << 12) | (S88Outdata[13] << 13) | (S88Outdata[14] << 14) | (S88Outdata[15] << 15);
S88DataOutShiftRegister2 = S88Outdata[16] | (S88Outdata[17] << 1) | (S88Outdata[18] << 2) | (S88Outdata[19] << 3) | (S88Outdata[20] << 4) | (S88Outdata[21] << 5) | (S88Outdata[22] << 6) | (S88Outdata[23] << 7) | (S88Outdata[24] << 8) | (S88Outdata[25] << 9) | (S88Outdata[26] << 10) | (S88Outdata[27] << 11) | (S88Outdata[28] << 12) | (S88Outdata[29] << 13) | (S88Outdata[30] << 14) | (S88Outdata[31] << 15);
S88DataOutShiftRegister3 = S88Outdata[32] | (S88Outdata[33] << 1) | (S88Outdata[34] << 2) | (S88Outdata[35] << 3) | (S88Outdata[36] << 4) | (S88Outdata[37] << 5) | (S88Outdata[38] << 6) | (S88Outdata[39] << 7) | (S88Outdata[40] << 8) | (S88Outdata[41] << 9) | (S88Outdata[42] << 10) | (S88Outdata[43] << 11) | (S88Outdata[44] << 12) | (S88Outdata[45] << 13) | (S88Outdata[46] << 14) | (S88Outdata[47] << 15);
S88DataOutShiftRegister4 = S88Outdata[48] | (S88Outdata[49] << 1) | (S88Outdata[50] << 2) | (S88Outdata[51] << 3) | (S88Outdata[52] << 4) | (S88Outdata[53] << 5) | (S88Outdata[54] << 6) | (S88Outdata[55] << 7) | (S88Outdata[56] << 8) | (S88Outdata[57] << 9) | (S88Outdata[58] << 10) | (S88Outdata[59] << 11) | (S88Outdata[60] << 12) | (S88Outdata[61] << 13) | (S88Outdata[62] << 14) | (S88Outdata[63] << 15);
S88DataOutShiftRegister5 = S88Outdata[64] | (S88Outdata[65] << 1) | (S88Outdata[66] << 2) | (S88Outdata[67] << 3) | (S88Outdata[68] << 4) | (S88Outdata[69] << 5) | (S88Outdata[70] << 6) | (S88Outdata[71] << 7) | (S88Outdata[72] << 8) | (S88Outdata[73] << 9) | (S88Outdata[74] << 10) | (S88Outdata[75] << 11) | (S88Outdata[76] << 12) | (S88Outdata[77] << 13) | (S88Outdata[78] << 14) | (S88Outdata[79] << 15);
S88DataOutShiftRegister6 = S88Outdata[80] | (S88Outdata[81] << 1) | (S88Outdata[82] << 2) | (S88Outdata[83] << 3) | (S88Outdata[84] << 4) | (S88Outdata[85] << 5) | (S88Outdata[86] << 6) | (S88Outdata[87] << 7) | (S88Outdata[88] << 8) | (S88Outdata[89] << 9) | (S88Outdata[90] << 10) | (S88Outdata[91] << 11) | (S88Outdata[92] << 12) | (S88Outdata[93] << 13) | (S88Outdata[94] << 14) | (S88Outdata[95] << 15);
S88DataOutShiftRegister7 = S88Outdata[96] | (S88Outdata[97] << 1) | (S88Outdata[98] << 2) | (S88Outdata[99] << 3) | (S88Outdata[100] << 4) | (S88Outdata[101] << 5) | (S88Outdata[102] << 6) | (S88Outdata[103] << 7) | (S88Outdata[104] << 8) | (S88Outdata[105] << 9) | (S88Outdata[106] << 10) | (S88Outdata[107] << 11) | (S88Outdata[108] << 12) | (S88Outdata[109] << 13) | (S88Outdata[110] << 14) | (S88Outdata[111] << 15);
S88DataOutShiftRegister8 = S88Outdata[112 ] | (S88Outdata[113] << 1) | (S88Outdata[114] << 2) | (S88Outdata[115] << 3) | (S88Outdata[116] << 4) | (S88Outdata[117] << 5) | (S88Outdata[118] << 6) | (S88Outdata[119] << 7) | (S88Outdata[120] << 8) | (S88Outdata[121] << 9) | (S88Outdata[122] << 10) | (S88Outdata[123] << 11) | (S88Outdata[124] << 12) | (S88Outdata[125] << 13) | (S88Outdata[126] << 14) | (S88Outdata[127] << 15);
S88DataOutShiftRegister9 = S88Outdata[128] | (S88Outdata[129] << 1) | (S88Outdata[130] << 2) | (S88Outdata[131] << 3) | (S88Outdata[132] << 4) | (S88Outdata[133] << 5) | (S88Outdata[134] << 6) | (S88Outdata[135] << 7) | (S88Outdata[136] << 8) | (S88Outdata[137] << 9) | (S88Outdata[138] << 10) | (S88Outdata[139] << 11) | (S88Outdata[140] << 12) | (S88Outdata[141] << 13) | (S88Outdata[142] << 14) | (S88Outdata[143] << 15);
S88DataOutShiftRegister10 = S88Outdata[144] | (S88Outdata[145] << 1) | (S88Outdata[146] << 2) | (S88Outdata[147] << 3) | (S88Outdata[148] << 4) | (S88Outdata[149] << 5) | (S88Outdata[150] << 6) | (S88Outdata[151] << 7) | (S88Outdata[152] << 8) | (S88Outdata[153] << 9) | (S88Outdata[154] << 10) | (S88Outdata[155] << 11) | (S88Outdata[156] << 12) | (S88Outdata[157] << 13) | (S88Outdata[158] << 14) | (S88Outdata[159] << 15);
attachInterrupt(1, LOAD, RISING);
}
That uses S88Output data above the range to which it was filled ?
Or is there some other code that fills S88Output ?
I tried to work with a boolean that became true when the interupt has been called and a function in the loop which is activated by that boolean. When the function was completed the boolean became false. But that doesn't work....
The sketch works likes this:
Clock raise -> send the first bit of the whole array.
Load raise -> move all the bits with one place. Since the first bit has been sended in the Clock function.
This code only works for 1 slave... The whole sketch need to read more slave and then will change the place of those 16 bits.
Cheers Dylan
@Robin2's comments still stand.
You should generally not put a lot of code in an ISR
Set a volatile flag in the ISR to tell the main loop that the data is ready etc, then in the main loop, wait for the flag and process the data as soon as its valid.
minitreintje:
Load raise -> move all the bits with one place. Since the first bit has been sended in the Clock function.
It is much easier to leave the data in place and simply point to the part to send next, then point to the next.
That way you only have to change one thing, the pointer, instead of the whole data block in bits.
Have you learned to use loops yet?
I like model trains and just out of curiosity I have done a little reading about S88 but I haven't been able to find a document that describes it at the level of the signal structure. Have you a link?
I get the impression that S88 is very similar to DCC - loco address, direction, speed etc.
Where are you getting the S88 signal from that your Arduino is trying to decode?
...R
I already build a lot with Arduino and I know how to use loops 
s88 is totally different from DCC... It's just a shift register that the cetral reads out. Euhm a good link is difficult since I searched for that for days before I begun with this. I found a code written in C where I took some parts of. That was the only good source that I found. The link: GitHub - dirkjankrijnders/S88NAVR: S88-N implementation for Atmega48 microcontrollers
Cheers,
Dylan
I didn't mean the S88 and DCC are similar in the sense that they might be interchangeable. I just meant that I suspect they may be similar at the level of detecting the bit-stream and converting it into bytes.
There is a lot of stuff in that Github link. Can you point me at a specific part to minimize the amount of stuff I need to read?
You didn't answer my question about what is producing the S88 signal that you are trying to interpret?
...R
Oops... The signal comes from my command station. The signal is just like an Arduino that reads a shift register. The stuff cames from s88ArduinoProMini file.
Cheers,
Dylan
Knows loops wrote this?
void LOAD() // Setup the S88data arrays / shift registers... Now it's ready to be send!
{
detachInterrupt(1); // Disable interrupt while doing some stuff with it.
S88DataOutShiftRegister1 = S88Outdata[0] | (S88Outdata[1] << 1) | (S88Outdata[2] << 2) | (S88Outdata[3] << 3) | (S88Outdata[4] << 4) | (S88Outdata[5] << 5) | (S88Outdata[6] << 6) | (S88Outdata[7] << 7) | (S88Outdata[8] << 8) | (S88Outdata[9] << 9) | (S88Outdata[10] << 10) | (S88Outdata[11] << 11) | (S88Outdata[12] << 12) | (S88Outdata[13] << 13) | (S88Outdata[14] << 14) | (S88Outdata[15] << 15);
S88DataOutShiftRegister2 = S88Outdata[16] | (S88Outdata[17] << 1) | (S88Outdata[18] << 2) | (S88Outdata[19] << 3) | (S88Outdata[20] << 4) | (S88Outdata[21] << 5) | (S88Outdata[22] << 6) | (S88Outdata[23] << 7) | (S88Outdata[24] << 8) | (S88Outdata[25] << 9) | (S88Outdata[26] << 10) | (S88Outdata[27] << 11) | (S88Outdata[28] << 12) | (S88Outdata[29] << 13) | (S88Outdata[30] << 14) | (S88Outdata[31] << 15);
S88DataOutShiftRegister3 = S88Outdata[32] | (S88Outdata[33] << 1) | (S88Outdata[34] << 2) | (S88Outdata[35] << 3) | (S88Outdata[36] << 4) | (S88Outdata[37] << 5) | (S88Outdata[38] << 6) | (S88Outdata[39] << 7) | (S88Outdata[40] << 8) | (S88Outdata[41] << 9) | (S88Outdata[42] << 10) | (S88Outdata[43] << 11) | (S88Outdata[44] << 12) | (S88Outdata[45] << 13) | (S88Outdata[46] << 14) | (S88Outdata[47] << 15);
S88DataOutShiftRegister4 = S88Outdata[48] | (S88Outdata[49] << 1) | (S88Outdata[50] << 2) | (S88Outdata[51] << 3) | (S88Outdata[52] << 4) | (S88Outdata[53] << 5) | (S88Outdata[54] << 6) | (S88Outdata[55] << 7) | (S88Outdata[56] << 8) | (S88Outdata[57] << 9) | (S88Outdata[58] << 10) | (S88Outdata[59] << 11) | (S88Outdata[60] << 12) | (S88Outdata[61] << 13) | (S88Outdata[62] << 14) | (S88Outdata[63] << 15);
S88DataOutShiftRegister5 = S88Outdata[64] | (S88Outdata[65] << 1) | (S88Outdata[66] << 2) | (S88Outdata[67] << 3) | (S88Outdata[68] << 4) | (S88Outdata[69] << 5) | (S88Outdata[70] << 6) | (S88Outdata[71] << 7) | (S88Outdata[72] << 8) | (S88Outdata[73] << 9) | (S88Outdata[74] << 10) | (S88Outdata[75] << 11) | (S88Outdata[76] << 12) | (S88Outdata[77] << 13) | (S88Outdata[78] << 14) | (S88Outdata[79] << 15);
S88DataOutShiftRegister6 = S88Outdata[80] | (S88Outdata[81] << 1) | (S88Outdata[82] << 2) | (S88Outdata[83] << 3) | (S88Outdata[84] << 4) | (S88Outdata[85] << 5) | (S88Outdata[86] << 6) | (S88Outdata[87] << 7) | (S88Outdata[88] << 8) | (S88Outdata[89] << 9) | (S88Outdata[90] << 10) | (S88Outdata[91] << 11) | (S88Outdata[92] << 12) | (S88Outdata[93] << 13) | (S88Outdata[94] << 14) | (S88Outdata[95] << 15);
S88DataOutShiftRegister7 = S88Outdata[96] | (S88Outdata[97] << 1) | (S88Outdata[98] << 2) | (S88Outdata[99] << 3) | (S88Outdata[100] << 4) | (S88Outdata[101] << 5) | (S88Outdata[102] << 6) | (S88Outdata[103] << 7) | (S88Outdata[104] << 8) | (S88Outdata[105] << 9) | (S88Outdata[106] << 10) | (S88Outdata[107] << 11) | (S88Outdata[108] << 12) | (S88Outdata[109] << 13) | (S88Outdata[110] << 14) | (S88Outdata[111] << 15);
S88DataOutShiftRegister8 = S88Outdata[112 ] | (S88Outdata[113] << 1) | (S88Outdata[114] << 2) | (S88Outdata[115] << 3) | (S88Outdata[116] << 4) | (S88Outdata[117] << 5) | (S88Outdata[118] << 6) | (S88Outdata[119] << 7) | (S88Outdata[120] << 8) | (S88Outdata[121] << 9) | (S88Outdata[122] << 10) | (S88Outdata[123] << 11) | (S88Outdata[124] << 12) | (S88Outdata[125] << 13) | (S88Outdata[126] << 14) | (S88Outdata[127] << 15);
S88DataOutShiftRegister9 = S88Outdata[128] | (S88Outdata[129] << 1) | (S88Outdata[130] << 2) | (S88Outdata[131] << 3) | (S88Outdata[132] << 4) | (S88Outdata[133] << 5) | (S88Outdata[134] << 6) | (S88Outdata[135] << 7) | (S88Outdata[136] << 8) | (S88Outdata[137] << 9) | (S88Outdata[138] << 10) | (S88Outdata[139] << 11) | (S88Outdata[140] << 12) | (S88Outdata[141] << 13) | (S88Outdata[142] << 14) | (S88Outdata[143] << 15);
S88DataOutShiftRegister10 = S88Outdata[144] | (S88Outdata[145] << 1) | (S88Outdata[146] << 2) | (S88Outdata[147] << 3) | (S88Outdata[148] << 4) | (S88Outdata[149] << 5) | (S88Outdata[150] << 6) | (S88Outdata[151] << 7) | (S88Outdata[152] << 8) | (S88Outdata[153] << 9) | (S88Outdata[154] << 10) | (S88Outdata[155] << 11) | (S88Outdata[156] << 12) | (S88Outdata[157] << 13) | (S88Outdata[158] << 14) | (S88Outdata[159] << 15);
attachInterrupt(1, LOAD, RISING);
}
Yes I know... I wrote it without a loop... I know how to use it but I didn't use it here.
Cheers,
Dylan
@GoForSmoke.. Yes we know... Its been pointed out already 
Back to the original question
The first place in the array S88DataOut[0] stays always 0 however the incoming data is 1.
As far as I can see in the code the only place that S88DataOut is assigned value is in this code
for (int i = 0; i < 16; i++)
{
S88Outdata[i] = S88Incomingdata[i];
}
So either there is a casting problem or S88Incomingdata[0] must be 0
minitreintje:
Oops... The signal comes from my command station.
Maybe I didn't ask the correct question ... but now I have a different one (following some more Googling about S88)
Are you trying to capture the S88 signal (all the HIGHs and LOWs) and transfer it to/from a wireless system without making any attempt to interpret the signal.
And (just to broaden my understanding) ...
I can find lots of web pages about the S88 hardware but nothing that describes what sort of data can be transmitted using S88 - in other words, what does the data mean and how is the meaning interpreted. Most of the pages I have seen seem to use S88 for train detection but none of them explains how the dection data is encoded. For example how does the PC know which detector has been triggered by a train?
...R
I will rewrite the sketch in the following days... With loops and etc.
Euhm the data is simple 1 or 0 depending on the detector is occupied or not. It's not encrypted or soemthing else. The slaves just report their digitalRead values when they are polled by the master with their address. The master puts these values in his shift register simulation. The 'protocol' is a shift register (I think you know how that works...) that is read by the command station. It replaces the original 4014 ICs that Marklin,... used.
I hope it is clear now
Cheers,
Dylan
minitreintje:
the data is simple 1 or 0 depending on the detector is occupied or not. It's not encrypted or soemthing else. The slaves just report their digitalRead values when they are polled by the master with their address.
Sorry, still not clear. How is an address encoded? Are you implying that every device can only respond with a 1 or a 0. Does the slave not have to send back its address as well as its data?
And you didn't answer this question
Are you trying to capture the S88 signal (all the HIGHs and LOWs) and transfer it to/from a wireless system without making any attempt to interpret the signal.
...R
Yes there isn't an address for every device... Weird hé 
The sequence of the bits is important (the reason why I used the << in the LOAD interrupt). The command station 'knows' when he starts with polling that he will get the first 8 bits from the first virtual shift register so he assigned them to 'S88 module 1', the following 8 bits will be shifted from the 2nd virtual shift register to the first one, the 8 bits from the 3rd virtual shift register will be shifted in the 2nd virtual shift register and the process repeat himself until all the data has been shifted to the first shift register.
I don't change the data when I send it from a slave to the master. The master just send all the data to the command station. That command station can use this data to activate a signal, turnout, ... or just send this data to the computer.
Cheers,
Dylan