"parameter" data type, some fixes, code optimization

[dukess]

Added: new data type: parameter. Right now it used in parsingStringToParameter only. "Default address" now is -1 instead 0 because zero is used for ADDR_HEIZ. This will more flexible for non-standard setups. Added: New JSON field "destination" for /JQ, /JC, /JK commands Added: destination address to debug output for /JS and /JR commands Fix: device family and variant saved after address changing. Fix: returning to default destination address for avg_parameters and log_parameters will work only at end of loop. Code optimization: code doublettes was moved to functions.

[dukess]

@fredlcore hi, can you do code review?

[fredlcore]

Thanks, @dukess, looks great! Just two questions: The reason for changing the default destination from 0 to -1 is because there could be a different default destination other than 0, correct? If that is so, then we have a breaking change in _config.h. That's fine with me, but wouldn't it be good then to also change the way the parameters like log_parameters[] are designed because you mentioned above that it would also be good to get rid of the *2+1-kind of calculations? I'm fine to do it the way you have coded it now, but if there is a more consistent way of doing it and we're having a breaking change now anyways, why not do it now?

Or will existing _config.h continue to work anyway because users having a different default destination other than 0 will have set a value other than 0 in log_parameters[] for destination anyway?

[dukess]

The reason for changing the default destination from 0 to -1 is because there could be a different default destination other than 0, correct?

Yes, you right.

Or will existing _config.h continue to work anyway because users having a different default destination other than 0 will have set a value other than 0 in log_parameters[] for destination anyway?

You right twice. :)

I'm fine to do it the way you have coded it now, but if there is a more consistent way of doing it and we're having a breaking change now anyways, why not do it now?

Because we didn't breaking changes yet, we can rewrite code before next release.

[fredlcore]

Ok, great :) - so it's ready for merging now?

[dukess]

Little bit later: i need to retest it with new commits.

[dukess]

Can't test cleanupDatalog() immediately but all other is ok.

[fredlcore]

If that's the case, we should stick to the code as it was, as using a different varialbe will decrease code readibility.

[dukess]

@DE-cr yes, you right. I rewrite solution and now it contain warning comment only.

[fredlcore]

Is this ready for a merge?

[fredlcore]

@dukess: Due to problems with some WiFi installations, I had to add a new variable to define the BSSID of the desired WiFi network. Since this results in a breaking change in BSB_LAN_config.h, it might be good to include your changes for the "cleaner" log parameters as well - if you have the time to do so in the next couple of days of course. If not, I'll do mine now and yours later, no problem.

[dukess]

@fredlcore got you! Can you merge this PR now? After some days for wide testing i will rewrite code for log parameters.

[dukess]

colleague, be patient, the second stage of changes will follow soon. :)

the previous time I immediately suggested radical changes, but they would have led to incompatibility between versions, which at that time was unacceptable for Frederik. Therefore, this stage was preparatory. Now it turns out that compatibility will still be broken by other changes, so I can implement my proposals without any problems.

-cr @.***> 17 мая 2023 г. 08:44:01 написал:

@DE-cr commented on this pull request. Ever since the introduction of the dest_addr handling I've been asking myself why you're doing these calculations: avg_parameters[token_counter] = atof(avg_token); avg_parameters[token_counter+1] = dest; avg_parameters[token_counter] = param.number; avg_parameters[token_counter+1] = param.dest_addr;

instead of using a struct { int number, dest_addr } for imo improved readability/maintainability. Now that you HAVE introduced this struct, I wonder even more why you're not using it (or at least not consistently)... — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: @.***>