Open Source Training Seminar FreePBX Paid Support

Ticket #2686 (closed Bugs: fixed)

Opened 6 months ago

Last modified 1 month ago

FOP limits parking lot to 5 slots

Reported by: fskrotzki Assigned to:
Priority: minor Milestone: 2.5
Component: Flash Operator Panel Version: 2.5-branch
Keywords: FOP Cc:
Confirmation: Need testing SVN Revision (if applicable):
Backend Engine: All Backend Engine Version: 1.2.26

Description (Last modified by fskrotzki)

The parking lot code changes from changeset 5642 have an additional condition in it that limits the number of slots to 5 which is a problem if you have more then 5. just need to remove that one conditional check (line 416 if the change set, remove " && $i < 5 "). See patch fix_parkinglot2.patch

Attachments

fix_parkinglot.patch (387 bytes) - added by fskrotzki on 02/18/08 13:43:17.
removes the 5 slot restriction on the parking lot display.
fix_parkinglot2.patch (0.8 kB) - added by fskrotzki on 03/12/08 10:38:20.
fix_parkinglot3.patch (0.8 kB) - added by fskrotzki on 07/16/08 14:08:05.
updated patch for latest release 2.4.1 code base
fix_parkinglot4.patch (0.8 kB) - added by fskrotzki on 07/17/08 10:22:54.
retrieve_op_conf_from_mysql.patch (2.4 kB) - added by Nick_Lewis on 07/24/08 08:21:25.
similar patch to #2641 that can be applied to head

Change History

02/18/08 13:43:17 changed by fskrotzki

  • attachment fix_parkinglot.patch added.

removes the 5 slot restriction on the parking lot display.

02/18/08 15:37:47 changed by p_lindheimer

  • confirmation changed from Unreviewed to Need Feedback.
  • milestone changed from Cut Line to 3.0.

I just tried it - it ends up putting the last 5 slots in the same place. Seems like something that balances between the parking slots, the trunks and the conf bridges needs to be put in place so they can move around.

02/18/08 18:47:48 changed by fskrotzki

If you create a parking lot with more then 5 spots (we have 9) then with for (my $i = 0 ; $i < $numberlots && $i < 5 ; $i++ ) it will stop when i = 5 so you never get to see the last 4 spots. Removing the second condition " && i < 5" solves this problem.

Granted we use a different layout then the new format that has come with 2.4 as we don't have queues so i have space to show 19 conferances and 9 spots in the parking lot both of which we do use.

02/18/08 19:03:12 changed by p_lindheimer

I understand what the change does - but when you apply it on the layout that is in 2.4 it just shows the last 5 slots in the same place (not necessarily in order either) so there is more that is needed to accommodate it.

02/19/08 00:35:07 changed by dani

Hi. The problem is that the actual layout doesn't allow more than five parkings. The solution would be to dynamically change the layout according to the number of conferences, queues, parkings and trunks configured, but it's more complicated.

02/19/08 04:57:29 changed by fskrotzki

I'd agree that that one section does limit it to the new layout. BUT none of the other sections apply this limit to there display area. If I change the display area for any section (extensions, trunks, queues, Conferances, parking lot) this one particular section is the only one that has a "arbitrary" limit. By arbitrary I mean it is exactly somebodys choice to limit that area which the code is then not applied to any other area.

I agree that the limits should be based on the actual area of the display region. But by removing it at least in my case of having 9 in a larger area I can see them (with the limit you can not and I have space for them). If you have more then the area allows the wrapping of the area is NOT that bad because when there are two stacked on top of each other the one with activity last will be updated and displayed on top of the one without activity (yes is stinks but I think it is a more workable solution). This is how it has worked in the past with extensions, etc...

If you want to apply a limit to keep it so it does not wrap: 1. be consistant and apply a limit to ALL areas at the same time. 2. make it user changable limit that is read from a file someplace so that you do not need to edit the code every time to fix it if you have a different layout.

The issue with the current limit implementaion is it is fixed in code and not easy for 95% of the people to find. IF you attempt to make minor changes to allow for more extensions or trunks you then gain a lot more screen space and this hard coded limit seems to make no since.

02/19/08 07:05:01 changed by p_lindheimer

well what is there was contributed as a result of a fix because of the way queues were chanced. I would also say that it is better than before since it has conferences and parking based on the config. The issue is, there is no one on the dev team who spends any time at all on FOP - so in order for FOP layouts and options to improve, it needs to be adopted by someone. Otherwise, if contributions are made that appear to work or improve it as this changed did, then we are happy to get them in. (Although I don't like the hard coded size and would be glad to see it go I believe that for most installations out there, 5 slots is probably adequate.

02/19/08 07:24:13 changed by fskrotzki

First off I agree totaly. The new functionality of having the queues work in this version again and now the conferances working which they have not at all in the past is a HUGE improvement and well worth including the code that was provided. When it was offered I was very excited about it.

I'm getting the hint on the dev... ;-) I'll see what I can do. The patch I provided just removes that one hard coded limit so that those of us who do have a different layout will have it work.

I can try and spend some additional time on it to see what I can contribute to making it better. There are several things I'd like changed and I'll start bothering my local Perl Guru for some help and see what I can do. If Perl only supported a simple include function so that like the rest of the system we could have a file that contained nothing but variable setting it would be so much easier. But since it does not I'll see if I can produce the code needed to read in those settings such that we can make the code more generic to anybody's setup and read from setup files so that it does not change basic setting each time it is updated. I'm not sure I can it dynamic (yes that would be the ultimate) but I can make it more aware of what the system needs and have it stay within those bounds.

First thing I'd do is have it read in the "fixed" variables at top for $extenpos, $trunkpos, $parkingpos, $confepos, and $queuepos as those I have to also redefine for my layout each and everytime the file is updated.

Send me a e-mail with some guidelines (where do you want the config fle, etc) and I'll get the Perl adjusted so it is better then it is currently.

02/19/08 07:48:21 changed by p_lindheimer

the other (preferable) option would be to find someone who wanted to change it to php. Step one, just a straight port, no functionality change. Step 2, improvements. It always hurts the head to switch to PHP and the goal is to drive all the remaining PERL code in FreePBX out. Not the FOP itself will ever be ported (as that is not our project). However - if we changed it to PHP, we could work towards incorporating more tightly within the code and maybe someone would even get around to making a simple GUI that would control the layout a bit. In any event - what ever is done will be welcome.

02/19/08 08:21:17 changed by fskrotzki

I agree a real UI for FOP that would allow people to do those settings would be the Nuts, BUT... until some things are pulled out of the code like placement limits and placement base variables (there are not many others, I named most of them in my previous comment) you can't easly start to make any UI to change settings as they are in the code and move all the time. That is the first thing I can offer to do as it will make my life easier thus gaining us all some way to move in that direction in the future.

03/12/08 10:37:30 changed by fskrotzki

  • description changed.

ok I fixed the origional code to now look at the $parkingpos variable and determine the number of usable slots and limit the loop to that total (i.e. the intent of the hard coded limit if you kept the origional layout). But now if a user goes in and re-defines the parking lot in any way it's new size it is taken into account properly.

03/12/08 10:38:20 changed by fskrotzki

  • attachment fix_parkinglot2.patch added.

06/15/08 01:26:45 changed by lazytt

  • owner deleted.

07/15/08 14:32:10 changed by lazytt

fskrotzki: would you please repost against trunk?

07/16/08 14:08:05 changed by fskrotzki

  • attachment fix_parkinglot3.patch added.

updated patch for latest release 2.4.1 code base

07/16/08 15:47:40 changed by p_lindheimer

OK I'm a bit confused, I changed parking lot to park on 70 and have 8 slots. With the patch applied I get:

Parking lots     Parked(72)
Parked(76)       Parked(73)
Parked(71)       Parked(74)

without the patch I get:

Parking lots     Parked(72)
Parked(70)       Parked(73)
Parked(71)       Parked(74)

also - why is Parked(70) listed at all, is that a bug? That is where you park people, not where anyone will ever be parked which starts at 71.

Anyhow- I'm confused as your patched didn't seem to increase the slots but left some confusing slots with the 76 in there)?

07/17/08 08:43:25 changed by fskrotzki

Hum. The showing of Parking slot 70 is a bug introduced from the previous layout change that I never noticed until you mentioned it. Glad somebody noticed I'll track it down and fix that also in next patch release which I'll do shortly.

The patches provided up to this point were only designed to allow the display of up to proper number of slots based on the display space given/defined at the top of the file (see lines 31 - 40 in retrieve_op_conf_from_mysql.conf). Previous to that with the changes that SME provided if you defined more slots then available display spots it wrapped around again reusing the first spot with the 6th spot, 2nd spot with 7th, etc in a fixed 5 spot layout. With the last change in code from SME he hard coded a max of 5 slots for display (and introduced the slot 70 issue). The problem with that is if you had more then 5 slots and allocated display space for more then 5 (We remove display of queue and use that gained space for increasing the display of conference rooms and parking slots) then you never saw the additional slots jsut empty display space.

If you increase the number of slots past what is capable of being displayed it should only show the number up to and including what is available. i.e. 5 slots.

I'll track down the bug of displaying parking spot 70 and make it start at the right place then the after the patch will have them in the proper order.

To test it this is what was done (assuming standard setup files): Edit op_buttons.cfg Find the first [rectangle] and find and change width and height to width=245 height=306

In second [rectangle] find x, y, width and height and change to x=738 y=32 width=245 height=306

Find third [rectangle] and comment it out

Find 2nd [LEGEND] and change x,y, and text to x=745 y=32 text=Parking lot

Comment out the 2nd [LEGEND]

Find 3rd [LEGEND] and change text to text=Conferences

in retrieve_op_conf_from_mysql.pl change to these values or insert them below existing lines: $confepos="42-51"; $queuepos=""; $parkingpos="62-71";

You will now have a 9 slot conference and parking lot and No queues.

07/17/08 10:13:19 changed by p_lindheimer

fskrotzki, can you modify your code so that it does not generate any more slots then you have space available. Then we could get it in. The fix for starting at 0 we can get in anyhow but unless you can make it so it does not wrap, the patch will introduce bugs for normal users who may have a number of parking slots beyond 5 but may not bother to change the layout and thus only see the first 5.

07/17/08 10:22:11 changed by p_lindheimer

btw - you have a bug in your fix, it won't show the last slot. I patched the slot 70 bug separately,

see: #2921

r6045 (2.5) and r6046 (2.4)

07/17/08 10:22:54 changed by fskrotzki

  • attachment fix_parkinglot4.patch added.

07/17/08 10:36:30 changed by fskrotzki

I just replaced the last patch #4. I saw that bug after I posted it. It get's the last slot now and will only fill to the space that is available.

FYI: The wrapping around to the beggining slot if more items exist then space allocated has been a bug for ALL catagories (trunks, queues, conferences, etc) since the beginning. I've just addressed it for parking lots with this patch because it was resticting to a hard coded amount.

The patch now will put them in proper order starting at the right number and fill until either you run out of slots available or no more parking lot numbers are defined which ever comes first.

The code to stop the looping in the slots needs to be (and can be) expanded to the other groups. I'll see if I have time today to address it and if so post it under a seperate bug, but it will be tight as I go on vacation at 5pm today.

07/17/08 17:44:00 changed by p_lindheimer

  • status changed from new to closed.
  • resolution set to fixed.

looks good, if you can get the 'wrapping' issue fixed in other parts of FOP we can put those in also. If you do it, open a new ticket if there is not one already opened.

r6048

07/24/08 04:26:37 changed by Nick_Lewis

For some reason the last line of the patch has not applied successfully.

07/24/08 05:01:20 changed by Nick_Lewis

If the last bit of the patch can be applied successfully i.e.

for (my $i = 1 ; $i <= $numberlots && $i <= $maxparkingslots ; $i++ ) {
$btn=get_next_btn($parkingpos,$btn);

rather than:

for (my $i = 1 ; $i <= $numberlots && $i <= 5 ; $i++ ) {
$btn=get_next_btn($parkingpos,$btn);

then I am sure that it will work fine. Its only disadvantage is that it requires a number of lines of code to define $maxparkingslots. A simpler solution may be to have just:

for (my $i = 1 ; $i <= $numberlots; $i++ ) {
$btn=get_next_btn($parkingpos,$btn);
if ($btn eq 0) {last;}

This one line test after get_next_btn can be used to prevent wrapping of other types of buttons too.

07/24/08 05:10:48 changed by Nick_Lewis

Hey it looks as if skurylo fixed all these issues with a patch that he submitted back in January. Please see ticket 2641.

07/24/08 06:51:11 changed by p_lindheimer

#2641 looks interesting, does it apply against trunk and has it been tested? Once that is the case, update that ticket and we can categorize it to Milestone 2.5 and see about getting it in.

as far as the slots being limited to 5, when I change it to $maxparkingslots all parking slots get removed from my flash. If that can be addressed and it doesn't overflow per the original issue I'll be glad to get it in. (and then who ever does it open a new ticket.

07/24/08 07:23:38 changed by Nick_Lewis

$maxparkingslots is coming out negative.

It looks to me as if the increments and decrements are the wrong way round in the patch. Perhaps fskrotzki meant:

my $maxparkingslots ;
$maxparkingslots = 0 ;
do
{
$btn=get_next_btn($parkingpos,$btn);
$maxparkingslots = $maxparkingslots + 1 ;
}
while ($btn != 0);
$maxparkingslots = $maxparkingslots - 1 ;

07/24/08 07:32:52 changed by p_lindheimer

  • confirmation changed from Need Feedback to Need testing.

have you tested and confirmed that change makes it work?

07/24/08 07:38:40 changed by Nick_Lewis

Regarding #2641 I have been using something extremely similar to that patch for 6 months with extensive testing and it has been fine. I am not sure if the patch from skurylo will still apply correctly to trunk. If not I will submit one that can. I think that this approach is preferable to getting the $maxparkingslots patch working.

07/24/08 07:58:08 changed by p_lindheimer

Nick, that would be great. btw - are you on IRC, can you get onto #freepbx-dev and try to get in contact?

07/24/08 08:21:25 changed by Nick_Lewis

  • attachment retrieve_op_conf_from_mysql.patch added.

similar patch to #2641 that can be applied to head

07/24/08 09:09:25 changed by Nick_Lewis

I have done some testing of the attached patch and it appears to operate successfully. With no changes to the "######## STYLE INFO #########" the number of parking positions is limited correctly to 5 but changing the "######## STYLE INFO #########" to take more positions for parking lots results in the correct number of additional parking lot button definitions being produced.

I think that if you make changes to the "######## STYLE INFO #########" section of retrieve_op_conf_from_mysql.pl then you will almost certainly need to adjust the rectangles and legends in op_buttons.cfg. I guess that once #2699 is properly implemented all these aspects will hang together a bit better.

07/24/08 09:17:43 changed by Nick_Lewis

Philippe - no go on IRC sorry

07/25/08 10:07:27 changed by p_lindheimer

  • version changed from 2.4-branch to 2.5-branch.

(In [6180]) closes #2945 and #2699 proper VM_PREFIX and dynamic screen space allocation, ref #2686 modified

Donate



Support
Download
Develop
Forums
News
Documentation
Paid Support
About

Paid Ads