Fix rotorez.c to accept config parameters before rig open.

[GeoBaltz]

Fix the non-conformant handling of config parameters between rot_init and rot_open. Fixes general application use of this rotator, and eliminates any need for special-casing.

[mdblack98]

That doesn't look like it will work. You are only saving the command and not the value.

[GeoBaltz]

Why doesn't my email reply show up here????

The command is the value - upper/lower case => on/off.

[mdblack98]

Turns our rotorez is not the only one that does this and it is much simpler to handle this in rotctl/rotctld with a 1-line change to add a rotor then to put this code in every backend. Plus you missed saving the value which is used for the command selection in the set_conf function. This does seem to be unique to rotor backends.

[GeoBaltz]

OK...so here's what I think the "best" solution is. A generic function in rig.c that can save parameter setting (both conf item and value) A generic function in rig.c that can process that list given a port. Backends that need to wait for _open then determine in the set_conf if the device is open and, if not, queue the conf=XXX string. Backends then look for the queue in the _open call and process it. Then this is just a few lines in each backend that needs it -- and actually the same few lines in them. Mike W9MDB

On Tuesday, June 11, 2024 at 10:33:33 AM CDT, George Baltz ***@***.***> wrote: On 6/11/24 8:20 AM, Michael Black wrote:   

Turns our rotorez is not the only one that does this and it is much simpler to handle this in rotctl/rotctld with a 1-line change to add a rotor then to put this code in every backend. Plus you missed saving the value which is used for the command selection in the set_conf function. This does seem to be unique to rotor backends. This is so wrong on so many levels I hardly know where to begin.  But lets go through them. "Plus you missed saving the value which is used for the command selection in the set_conf function." This gets me the most - I save the command that would be sent to the rotator, not anything from the set_conf call.  It's already been encoded with the value - upper case for set, lower case for clear.   I just send the same command to the rotator a little bit later. "Turns our rotorez is not the only one that does this and it is much simpler to handle this in rotctl/rotctld with a 1-line change to add a rotor then to put this code in every backend." Yes, it might be simpler - but wrong.  It doesn't fix the root cause, that rotorez_set_conf() doesn't agree with the (unwritten) Hamlib specifications.  That means an application(not just rotctl[d]) will call rot_set_conf() before rot_open(), expecting the value will be set for future use, whenever it is referenced.  Moreover, it could mix frontend and backend parameters in the same set of calls.  They have to be handled similarly. ISTM that this is the very basic reason that Hamlib exists - to provide a completely consistent sequence of calls to handle any parameter of any device. If there are others, I'll fix those too.  Why? Because I don't like incomplete fixes. It only took half an hour to write the code once I figured out where things were, so lets fix it right.

…

   Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.Message ID: ***@***.***>   

_______________________________________________ Hamlib-developer mailing list ***@***.*** https://lists.sourceforge.net/lists/listinfo/hamlib-developer

[GeoBaltz]

That's certainly one solution that covers this case.  But I was thinking of something more general - more than just conf entries. I still like queuing only the actual rig command, as that means the parsing and error checking/reporting is out of the way, but maybe there are other uses.  Add a type field and you can get a generic deferred work queue.  Might be useful in other cases, like some of the thread-safety issues. Let's actually design this, not just hack something that's not as useful or as foolproof as it could be.

…

On 6/11/24 11:45 AM, Black Michael wrote: OK...so here's what I think the "best" solution is. A generic function in rig.c that can save parameter setting (both conf item and value) A generic function in rig.c that can process that list given a port. Backends that need to wait for _open then determine in the set_conf if the device is open and, if not, queue the conf=XXX string. Backends then look for the queue in the _open call and process it. Then this is just a few lines in each backend that needs it -- and actually the same few lines in them. Mike W9MDB On Tuesday, June 11, 2024 at 10:33:33 AM CDT, George Baltz ***@***.***> wrote: On 6/11/24 8:20 AM, Michael Black wrote: > Turns our rotorez is not the only one that does this and it is much simpler to handle this in rotctl/rotctld with a 1-line change to add a rotor then to put this code in every backend. Plus you missed saving the value which is used for the command selection in the set_conf function. This does seem to be unique to rotor backends. This is so wrong on so many levels I hardly know where to begin.  But lets go through them. "Plus you missed saving the value which is used for the command selection in the set_conf function." This gets me the most - I save the command that would be sent to the rotator, not anything from the set_conf call.  It's already been encoded with the value - upper case for set, lower case for clear.   I just send the same command to the rotator a little bit later. "Turns our rotorez is not the only one that does this and it is much simpler to handle this in rotctl/rotctld with a 1-line change to add a rotor then to put this code in every backend." Yes, it might be simpler - but wrong.  It doesn't fix the root cause, that rotorez_set_conf() doesn't agree with the (unwritten) Hamlib specifications.  That means an application(not just rotctl[d]) will call rot_set_conf() before rot_open(), expecting the value will be set for future use, whenever it is referenced.  Moreover, it could mix frontend and backend parameters in the same set of calls.  They have to be handled similarly. ISTM that this is the very basic reason that Hamlib exists - to provide a completely consistent sequence of calls to handle any parameter of any device. If there are others, I'll fix those too.  Why? Because I don't like incomplete fixes. It only took half an hour to write the code once I figured out where things were, so lets fix it right. > > > Reply to this email directly, view it on GitHub, or unsubscribe. > You are receiving this because you authored the thread.Message ID: ***@***.***> > _______________________________________________ Hamlib-developer mailing list ***@***.*** https://lists.sourceforge.net/lists/listinfo/hamlib-developer

[mdblack98]

Let's not make this backend-dependent.  Requiring the queue to know the backend's commands means lots of code parsing the back-end...which is completely unnecessary.  And yes...parameter checking is delayed another few milliseconds. I'm looking at long-term maintenance so having a couple of lines of code in rigctl.c and rigctld.c and a couple more in each backend that needs is much easier to maintain than dozens of lines duplicating what is already done in the backends. The idea I gave can also support any X=Y setting for conf or parm settings for example. Mike W9MDB On Tuesday, June 11, 2024 at 01:29:47 PM CDT, GeoBaltz ***@***.***> wrote: That's certainly one solution that covers this case.  But I was thinking of something more general - more than just conf entries. I still like queuing only the actual rig command, as that means the parsing and error checking/reporting is out of the way, but maybe there are other uses.  Add a type field and you can get a generic deferred work queue.  Might be useful in other cases, like some of the thread-safety issues. Let's actually design this, not just hack something that's not as useful or as foolproof as it could be.

On 6/11/24 11:45 AM, Black Michael wrote: OK...so here's what I think the "best" solution is. A generic function in rig.c that can save parameter setting (both conf item and value) A generic function in rig.c that can process that list given a port. Backends that need to wait for _open then determine in the set_conf if the device is open and, if not, queue the conf=XXX string. Backends then look for the queue in the _open call and process it. Then this is just a few lines in each backend that needs it -- and actually the same few lines in them. Mike W9MDB On Tuesday, June 11, 2024 at 10:33:33 AM CDT, George Baltz ***@***.***> wrote: On 6/11/24 8:20 AM, Michael Black wrote: > Turns our rotorez is not the only one that does this and it is much simpler to handle this in rotctl/rotctld with a 1-line change to add a rotor then to put this code in every backend. Plus you missed saving the value which is used for the command selection in the set_conf function. This does seem to be unique to rotor backends. This is so wrong on so many levels I hardly know where to begin.  But lets go through them. "Plus you missed saving the value which is used for the command selection in the set_conf function." This gets me the most - I save the command that would be sent to the rotator, not anything from the set_conf call.  It's already been encoded with the value - upper case for set, lower case for clear.   I just send the same command to the rotator a little bit later. "Turns our rotorez is not the only one that does this and it is much simpler to handle this in rotctl/rotctld with a 1-line change to add a rotor then to put this code in every backend." Yes, it might be simpler - but wrong.  It doesn't fix the root cause, that rotorez_set_conf() doesn't agree with the (unwritten) Hamlib specifications.  That means an application(not just rotctl[d]) will call rot_set_conf() before rot_open(), expecting the value will be set for future use, whenever it is referenced.  Moreover, it could mix frontend and backend parameters in the same set of calls.  They have to be handled similarly. ISTM that this is the very basic reason that Hamlib exists - to provide a completely consistent sequence of calls to handle any parameter of any device. If there are others, I'll fix those too.  Why? Because I don't like incomplete fixes. It only took half an hour to write the code once I figured out where things were, so lets fix it right. > > > Reply to this email directly, view it on GitHub, or unsubscribe. > You are receiving this because you authored the thread.Message ID: ***@***.***> > _______________________________________________ Hamlib-developer mailing list ***@***.*** https://lists.sourceforge.net/lists/listinfo/hamlib-developer

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you modified the open/close state.Message ID: ***@***.***>