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

Drop some compatibility things #310

Merged
merged 2 commits into from Dec 28, 2022
Merged

Drop some compatibility things #310

merged 2 commits into from Dec 28, 2022

Conversation

kk7ds
Copy link
Owner

@kk7ds kk7ds commented Dec 25, 2022

  • Drop py2 tox targets
  • Delete chirp.ui module

@kk7ds kk7ds changed the base branch from py3 to master December 27, 2022 20:56
@vishwin
Copy link
Contributor

vishwin commented Dec 27, 2022

As mentioned on the mailing list, the GTK interface still works perfectly fine (after accounting for certain imports moved to wxui)

image
(PyGObject3 presents itself as PyGTK 2.99.0 because compatibility code)

@mfncooper
Copy link
Contributor

@vishwin The decision was made around 3 years ago to move to wxPython for the UI. This gives us a high quality cross-platform UI that provides users with a native look and feel on their choice of OS, and thus a better user experience on each platform. At the same time, developers can be assured that UI changes will be reflected appropriately on all supported platforms.

It does not make sense to retain the old UI and attempt to maintain parity between an old and a new interface. That would cause a lot of extra work for developers for no good reason, as well as create confusion for end users, authors, and packagers of Chirp. The sooner we drop the old UI, the better, so that the future direction is clear to everyone.

@kk7ds
Copy link
Owner Author

kk7ds commented Dec 28, 2022

Agree with @mfncooper .

Additionally, as you noted, you had to make changes to get it to even start after recent changes. Unless you did a lot more work, the interface between the UI and the query sources has changed such that more work would be required to make it actually work for things like RR, RepeaterBook, etc. Maintaining both sides of that for two UIs is not something I want to do, and I don't want to keep a continually-eroding UI in the tree and certainly don't want any of the packagers handing that to users who have the expectation that it works.

I also know for a fact that the serial port behavior in the old UI doesn't match what we're doing in the new one, such that I'm pretty sure that drivers that have been migrated to work with bytes serial but not bytes memmap will crash when handed the CompatSerial thing.

Further, the pygtk compat thing is a shim that exists purely to get off of it:

https://github.com/GNOME/pygobject/blob/master/pygtkcompat/pygtkcompat.py#L27-L28

That means that keeping something around based on it goes against their recommendation, which means work to keep it alive is pointless, IMHO.

@vishwin
Copy link
Contributor

vishwin commented Dec 28, 2022

All I did was change some imports to reflect new locations. I'm not sure if the Baofeng/BTECH/QYT drivers migrated to the new behaviour yet, but they didn't crash in either direction.

provides users with a native look and feel on their choice of OS, and thus a better user experience on each platform.

It's not quite native on GTK despite wxWidgets specifically targeting it, and up until now the cut/copy-paste workflow (particularly multi-select) didn't work at all on wxui. Perhaps I still have a lingering bad taste from when I tried a very early iteration of wxui almost three years ago, but things have gotten a lot better now, so don't let my noise create pause from removing the GTK interface.

@kk7ds
Copy link
Owner Author

kk7ds commented Dec 28, 2022

up until now the cut/copy-paste workflow (particularly multi-select) didn't work at all on wxui.

Multi-select copy and paste has been implemented since the very first wx commit in 2019:

f4ad30d

as seen here:

chirp/chirp/wxui/memedit.py

Lines 324 to 358 in f4ad30d

def cb_copy(self, cut=False):
rows = self._grid.GetSelectedRows()
offset = self._features.memory_bounds[0]
mems = [self._radio.get_memory(row + offset) for row in rows]
data = wx.CustomDataObject(common.CHIRP_DATA_MEMORY)
data.SetData(pickle.dumps(mems))
for mem in mems:
if cut:
self._radio.erase_memory(mem.number)
mem = self._radio.get_memory(mem.number)
self.refresh_memory(mem)
return data
def _cb_paste_memories(self, mems):
try:
row = self._grid.GetSelectedRows()[0]
except IndexError:
LOG.info('No row selected for paste')
return
for mem in mems:
mem.empty = False
mem.number = row + self._features.memory_bounds[0]
row += 1
self._radio.set_memory(mem)
self.refresh_memory(mem)
def cb_paste(self, data):
if data.GetFormat() == common.CHIRP_DATA_MEMORY:
mems = pickle.loads(data.GetData().tobytes())
self._cb_paste_memories(mems)
elif data.GetFormat() == wx.DF_UNICODETEXT:
LOG.debug('FIXME: handle pasted text: %r' % data.GetText())
else:
LOG.warning('Unknown data format %s' % data.GetFormat().Type)

And has been working for me (at least) ever since. More work as been done there, so perhaps there was a bug preventing it for working for you or something. And as I said, if there's a reproducible issue still lingering, please point it out and I'll work on it.

so don't let my noise create pause from removing the GTK interface.

Thanks :)

I want to make sure tonight's build goes well with as much change as the py3->master commit generated. I'll plan to rebase and merge this tomorrow.

@KC9HI
Copy link
Contributor

KC9HI commented Dec 28, 2022

I guess I don't understand what GTK would bring to CHIRP that wx won't/can't. I have been using the legacy version of CHIRP for over 10 years. I have used CHIRP a lot over this time (I have over 100 CHIRP supported radios). So I am very familiar with how the legacy CHIRP looks and works. I've been using the Py3 CHIRP build since the last week of October. I've been using it nearly ever day and for many hours each day. It has been working great for me. It has pretty much become my daily driver. I don't really have much need to use the legacy CHIRP any more except to provide support to those that still are.

I am happy to go with py3 and leave py2 behind.
I am happy to go with git and leave Mercurial behind.

The work has already been done switching to wxPython for the UI.
I don't understand why I wouldn't want to leave GTK behind as well?

@vishwin
Copy link
Contributor

vishwin commented Dec 28, 2022

When Python 2 became a persona non grata on my operating system, I privately kept and patched a py3 mercurial commit from February 2020, without ever updating the drivers, but continuing to use the GTK interface but with PyGObject3 (instead of the deprecated pygtk2), because wxui at the time was effectively unusable for my workflow.

I guess I don't understand what GTK would bring to CHIRP that wx won't/can't.

At least on my setup, the GTK interface consumes about 50 MiB less memory than wxui. Additionally, for those on systems with a tradition of building everything as practicable from source, the wxPython (± wxWidgets depending on linking strategy) toolkit itself is a fairly taxing build compared to GTK. Neither consideration is that significant compared to other maintenance considerations, however. (Also not to mention that apparently the vast majority of users are on Windows, but I digress…)

This is more of a personal belief, that would have been more appropriate two or three years ago, that a wholesale conversion to wxPython was unnecessary, and adaption to PyGObject3 (ie converting pygtk2 constructs to those of PyGObject3) would have been better.

Again, I'm not pressing against the GTK interface's removal, but rather to illustrate that mileages will vary, especially with fit and finish, with the same exact program running on different combinations of platforms and toolkits.

@kk7ds
Copy link
Owner Author

kk7ds commented Dec 28, 2022

The GTK interface is substantially slower for me on every platform I've used it on. I haven't looked at memory usage really, but in terms of speed, there is no comparison for me. The entire reason the GTK interface has the memory range selector (which confuses the users a LOT) is specifically because it's so incredibly slow with 1000+ memories loaded and reducing the number we render is necessary. I've loaded the entire NA repeaterbook database into the wxui grid and it's fast. With two large radio images loaded, it's using half the memory that my terminal is, and is 19th from the top of everything running on my laptop right now. I can live with that :)

The GTK row store is not well-behaved in windows (for people familiar with windows) and 0% of the widgets are natively rendered, which means assistive technologies see only a blank window. Windows users represent 93% of the user base. GTK has always been unstable on macos (see all the apple-generated crash reports in the issues), doesn't look native, doesn't render well on HiDPI (which is every mac now) and is a bear to build the toolchain. Macos users represent 5% of the user base. That leaves 2% for various platforms, which includes Linux and BSD. I've never been a windows user, and have more Linux in my house than anything else, but it's hard to argue with those numbers, and that's why wx seems like a slam dunk to me.

Anyway, like @mfncooper said, this was decided three years ago, but the above is just some of my reasoning at least. It sounds like we're all in agreement here about what's happening (even if not ideal for everyone) and that keeping both isn't the solution. I appreciate the understanding that there are lots of factors here :)

This removes the legacy GTK GUI from the tree. This is not being
maintained, doesn't work very well, and I'm tired of accidentally
editing files here when I mean to edit wxui/*. When we announce the
chirp-next stuff to the users, I don't want packagers to be confused
about which version to use, what the required dependencies are, etc.
@kk7ds kk7ds merged commit b905468 into master Dec 28, 2022
@kk7ds kk7ds deleted the dropcompat branch December 28, 2022 16:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants